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

nfebe
Copy link
Contributor

@nfebe nfebe 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 /

@AndyScherzinger AndyScherzinger mentioned this pull request Nov 29, 2023
3 tasks
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 <[email protected]>
Signed-off-by: nextcloud-command <[email protected]>
@AndyScherzinger
Copy link
Member

/compile /

Signed-off-by: nextcloud-command <[email protected]>
@susnux
Copy link
Contributor

susnux commented Nov 29, 2023

drone error unrelated

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

nfebe commented Nov 29, 2023

/backport to stable28

@AndyScherzinger AndyScherzinger added this to the Nextcloud 29 milestone Nov 29, 2023
nfebe 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