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

Handle close GlobalSearchModal gracefully #41792

Merged
merged 2 commits into from
Nov 29, 2023
Merged

Handle close GlobalSearchModal gracefully #41792

merged 2 commits into from
Nov 29, 2023

Conversation

Fenn-CS
Copy link
Contributor

@Fenn-CS Fenn-CS commented Nov 28, 2023

Sorts out re-opening errors, such as needing two clicks after clicking outside, or modal flashes.


The current close infrastructure modifies a prop which has no real effect aside bugs.

In addition, calling the NcModal.close() as the primary way to close the search modal instead of using the states defined in GlobalSearch view causing re-open bugs (Modal cannot open, needs to click twice, and other weird stuff).

Contributes to #41381

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Code looks good. Didn't test

@ChristophWurst
Copy link
Member

/backport to stable28

Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

I think that using the .sync modifier can help in such cases: https://v2.vuejs.org/v2/guide/components-custom-events.html#sync-Modifier
But code looks good like that

@AndyScherzinger
Copy link
Member

/compile amend /

The current close infrastructure modifies a prop which has
 no real effect aside bugs.

In addition, calling the `NcModal.close()` as the primary way to
 close the search modal instead of using the states defined in `GlobalSearch` view
 causing re-open bugs (Modal cannot open, needs to click twice, and other weird stuff).

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@AndyScherzinger
Copy link
Member

/compile /

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@susnux
Copy link
Contributor

susnux commented Nov 29, 2023

drone error unrelated

@susnux susnux merged commit b213fc7 into master Nov 29, 2023
40 of 41 checks passed
@susnux susnux deleted the avoid-mutating-prop branch November 29, 2023 15:27
Fenn-CS pushed a commit that referenced this pull request Nov 29, 2023
Handle close GlobalSearchModal gracefully
@Fenn-CS
Copy link
Contributor Author

Fenn-CS commented Nov 29, 2023

/backport to stable28

@AndyScherzinger AndyScherzinger added this to the Nextcloud 29 milestone Nov 29, 2023
Fenn-CS added a commit that referenced this pull request Nov 30, 2023
[stable28]  Handle close GlobalSearchModal gracefully #41792
@blizzz blizzz mentioned this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants