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 for NcActionInput type multiselect #3760

Merged
merged 2 commits into from
Feb 20, 2023

Conversation

raimund-schluessler
Copy link
Contributor

@raimund-schluessler raimund-schluessler commented Feb 15, 2023

This replaces the deprecated NcMultiselect with the new NcSelect component for the NcActionInput component of type multiselect.

Although it got better (the dropdown wasn't visible at all before), Like before, there currently still is a z-index issue for the select dropdown. This is because the NcActions component has a z-index: 100000 and the select dropdown only got 9999. Fixing this is tricky at the moment, because the dropdown gets appended to body without a custom class that could be targeted, which means we can only set the z-index for all select dropdowns globally, which I don't know whether we want this. The proper solution would be to enhance NcSelect to allow to set a custom dropdown class, but that needs an upstream fix.

Before After
select closed Screenshot 2023-02-15 at 21-48-03 Nextcloud Vue Style Guide Screenshot 2023-02-15 at 21-47-11 Nextcloud Vue Style Guide
select open image Screenshot 2023-02-15 at 22-17-25 Nextcloud Vue Style Guide

I propose we go ahead despite the z-index issue, since it is already better than before.

Fixes 1/3 of #3743.

@raimund-schluessler raimund-schluessler added 3. to review Waiting for reviews feature: actions Related to the actions components feature: select Related to the NcSelect* components technical debt labels Feb 15, 2023
@raimund-schluessler raimund-schluessler marked this pull request as ready for review February 15, 2023 21:02
@raimund-schluessler raimund-schluessler changed the title Use NcSelect for NcActionInput type multiselect Use NcSelect for NcActionInput type multiselect Feb 15, 2023
@susnux
Copy link
Contributor

susnux commented Feb 15, 2023

Does not change much on the functionality (still that z-index issues), but from the styling it now looks different.
Could we adjust the border color to match up with the other input types?

Same with the border radius, it looks rounder than the other input types, but this was even before so not related to this PR (it just catch my eye).

@raimund-schluessler
Copy link
Contributor Author

Does not change much on the functionality (still that z-index issues), but from the styling it now looks different. Could we adjust the border color to match up with the other input types?

Same with the border radius, it looks rounder than the other input types, but this was even before so not related to this PR (it just catch my eye).

I would say, the other inputs need to be adjusted. Ideally, we should use an NcTextField here, then the borders and their radii should look the same. I would rather not touch the NcSelect styling, though. I can do that in a follow-up PR later.
The z-index problem is annoying, but it still works, since the actions menu simply shows a scrollbar and you can still select the options.

Copy link
Contributor

@Pytal Pytal left a comment

Choose a reason for hiding this comment

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

🎨

src/components/NcActionInput/NcActionInput.vue Outdated Show resolved Hide resolved
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Copy link
Contributor

nickvergessen commented Feb 20, 2023

Added a doc sample with object support:
grafik

@raimund-schluessler raimund-schluessler added this to the 7.7.0 milestone Feb 20, 2023
@nickvergessen nickvergessen merged commit 10a63d4 into master Feb 20, 2023
@nickvergessen nickvergessen deleted the fix/3743/action-input-select branch February 20, 2023 13:30
@raimund-schluessler raimund-schluessler added bug Something isn't working accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Feb 21, 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 accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working feature: actions Related to the actions components feature: select Related to the NcSelect* components technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants