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

Use NcSelect in NcRichtext #4247

Merged
merged 4 commits into from Jun 23, 2023

Conversation

julien-nc
Copy link
Contributor

refs #3743

Switch to NcSelect in NcProviderList and NcSearch from NcRichText.

I couldn't figure out a way to avoid an issue with NcSearch. When selecting a "Load more..." item, even if we actually select the item while searching for more results, the selection jumps to the top and then back to the "more" item.

Not so pretty...but not so bad.

vokoscreenNG-2023-06-22_13-27-31.mp4

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
@julien-nc julien-nc added 3. to review Waiting for reviews technical debt feature: richtext Related to the richtext component labels Jun 22, 2023
@julien-nc julien-nc added this to the 8.0.0 milestone Jun 22, 2023
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
@julien-nc julien-nc force-pushed the enh/3743/use-ncselect-in-ncrichtext branch from 42fa122 to 218f2ad Compare June 22, 2023 11:38
@Pytal
Copy link
Contributor

Pytal commented Jun 22, 2023

I couldn't figure out a way to avoid an issue with NcSearch. When selecting a "Load more..." item, even if we actually select the item while searching for more results, the selection jumps to the top and then back to the "more" item.

lmk if it works with nextcloud-deps/vue-select#20 :)

Copy link
Contributor

@raimund-schluessler raimund-schluessler left a comment

Choose a reason for hiding this comment

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

Looks good. Just the timeout issue makes me wondering.

@julien-nc
Copy link
Contributor Author

lmk if it works with nextcloud-deps/vue-select#20 :)

Interesting. I'll check if it helps.

…arch

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
…issue when 'searching for more' in NcRichtext's NcSearch

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
@julien-nc
Copy link
Contributor Author

@Pytal Much better with resetFocusOnOptionsChange 💙 Looking forward to it.
It's used and set to false in NcSearch even if it's not in NcSelect yet. Are we fine with that?

vokoscreenNG-2023-06-23_11-07-41.mp4

@julien-nc
Copy link
Contributor Author

Added another commit to make sure the option keys are unique. (It's just setting the id attribute of the option objects, I didn't wanna define the getOptionKey prop).

@Pytal
Copy link
Contributor

Pytal commented Jun 23, 2023

@Pytal Much better with resetFocusOnOptionsChange 💙 Looking forward to it. It's used and set to false in NcSearch even if it's not in NcSelect yet. Are we fine with that?

Great! We can merge the bump #4264 too!

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

👍

@raimund-schluessler raimund-schluessler merged commit 68b3f9e into master Jun 23, 2023
16 checks passed
@raimund-schluessler raimund-schluessler deleted the enh/3743/use-ncselect-in-ncrichtext branch June 23, 2023 18:03
@skjnldsv skjnldsv mentioned this pull request Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews feature: richtext Related to the richtext component technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants