-
Notifications
You must be signed in to change notification settings - Fork 95
[DRAFT] feat: add NcFormBoxSelectNative #7838
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Grigorii K. Shartsev <[email protected]>
64589e8 to
772cac3
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7838 +/- ##
=======================================
Coverage 51.41% 51.41%
=======================================
Files 96 96
Lines 3147 3147
Branches 868 867 -1
=======================================
Hits 1618 1618
Misses 1279 1279
Partials 250 250 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5649568 to
8b89c19
Compare
8b89c19 to
3e07e16
Compare
kra-mo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice, thank you :)
| position: absolute; | ||
| inset: 0; | ||
| margin: 0; | ||
| height: auto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only thing is that this makes it so that on macOS, the currently selected item's label is aligned to the top of the component, but in reality, the label is on the bottom.
Not sure if there is an easy fix for this though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you share a screenshot? I'm not sure I get what you mean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your screenshot in the issue description also exhibits this issue. Basically, select on macOS (and to an extent iOS) moves the list up/down depending on the selected item to align the currently selected list item with the component.
Essentially, the label in the list and the label in the component shouldn't move when opening the list and should be perfectly (or at least closely) aligned.
Example of how it should look, seeing "Goldfish" only once:
whereas for us, comparatively, it is shifted up and you see "Never" twice:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example of how it should look, seeing "Goldfish" only once
But in the example the select is small, and our button is 40px.
If the entire button just were a select on the exact same place, it would look exactly the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not shifted relative to macOS's select's picker position
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better if I explain it this way: since the component is taller than list items, right now the list is basically aligned as if there was a less tall invisible select that was top-aligned.
But since our label is at the bottom, it should act like there is a less tall invisible select that is bottom-aligned instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the entire button just were a select on the exact same place, it would look exactly the same.
And yes, but it should basically be as if only the bottom half were a select, since that is where our label is, not the top half. Not sure if this is easily doable though.
(Maybe if the text in the invisible select could be also bottom-aligned? But maybe it's not smart enough to pick up on that even if it is possible.)
|
Is this the new way to go with components? Especially because we did have special components for special sections before, but they got unmaintained and caused weird design differences between apps (when to use what and why does it look like this in one place but directly next to it it looks different). So we unified the components to generic components. Here for example I do not really understand why a select in app settings or other form like locations should look and behave very different from e.g. the main app where the NcSelect would be used. Also in the past it was the goal to have a consistent webui experience independent from the browser or OS, but with such native components we will loose the Nextcloud consistent design and get different look if you e.g. just switch devices. |
I don't think that this is really an issue. Many websites use the native select component even when they don't have browser-specific adjustments. I see this as the same as using the native font or not overriding the native context menu, which basically has the same styling as select. |
I do understand what you mean, but to me, there is a pretty clear semantic difference that I can see:
And I don't think that it being form-specific should be an issue. I can't think of many non-form places where selection is needed, including in the main app. Maintenance is of course another concern, and not my area. |
☑️ Resolves
.showPicker()is not supported on Safari https://caniuse.com/wf-show-picker-selectvue-select(I'd avoid it, it is complicated, heavy and too much)NcActionsusingNcFormBoxButtonas the trigger🖼️ Screenshots
Chromium - Windows
Firefox Windows
Chromium macOS
Safari macOS
🏁 Checklist
stable8for maintained Vue 2 version or not applicable