Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the modal window functionality #48

Merged
merged 17 commits into from
Oct 9, 2024
Merged

Conversation

Artem-Semenov-dev
Copy link
Contributor

This PR implements the ModalWindow component and extends the API of the AppWindow class allowing the creation and management of modal dialogs within the application.

@Artem-Semenov-dev Artem-Semenov-dev self-assigned this Oct 7, 2024
@Artem-Semenov-dev Artem-Semenov-dev marked this pull request as ready for review October 7, 2024 11:44
Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Artem-Semenov-dev please see my comments.

* The responsibility for handling the appearance, layout, and structure
* of the modal window is delegated to the provided composable content block.
*
* @param onCancel The callback triggered when the user clicks outside the modal window.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of the modal window is that no matter where user clicks outside the window borders, it stays on top. Otherwise, it's not modal.

The only way to close the window is to act explicitly. E.g. press some "Close" button.

In terms of the domain-related processes, there should be a way to confirm such a closure with the user first. Otherwise, the outcome of presumably active process (supplied on the UI by this window) is unclear.

@Artem-Semenov-dev Artem-Semenov-dev merged commit 22c6d65 into master Oct 9, 2024
5 checks passed
@Artem-Semenov-dev Artem-Semenov-dev deleted the modal-window branch October 9, 2024 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants