-
Notifications
You must be signed in to change notification settings - Fork 85
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
Change styles for focused Select #4728
Change styles for focused Select #4728
Conversation
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.
Dear @raimund-schluessler unfortunately i can't reproduce it: could you please try again? @marcoambrosini could you please help me a bit to find box-shadow styles? |
It seems to be fixed. I don't see it anymore either. However, the border at the bottom is still blue when the select is opened to the top. See this screenshot from the docs: |
@raimund-schluessler styles of my local styleguide instance are broken ;( I guess i've fixed an issue, could you please re-review? Thanks a lot! |
The border is fixed now. 👍 |
6b36f3f
to
bf0f43f
Compare
@JuliaKirschenheuter Sorry, I just saw there is another box-shadow when the dropdown opens to the top. This line would also need to go:
And could you please rebase your branch so that it includes all latest changes from master? I think there were some recent adjustments of |
Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
bf0f43f
to
f77b65f
Compare
@@ -1182,8 +1184,10 @@ body { | |||
} | |||
|
|||
.vs__dropdown-menu { | |||
border-color: var(--color-primary-element) !important; | |||
border-color: var(--color-main-text) !important; | |||
outline: 2px solid var(--color-main-background); |
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.
This outline adds a not so nice gap between the select toggle and the dropdown-menu. But I am not sure we can fix this.
More obvious with adjusted outline color
@susnux Any idea here?
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.
well, without this outline
it looks odd too like from your video:
After.mp4
I think it is better to keep outline
. And all other improvements we can do after a11y certification
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.
well, without this
outline
it looks odd too like from your video:
That's unfortunately true. One could fix it with an additional container around the dropdown and some CSS including a margin-top: -2px;
, but that would require adjusting @nextcloud/vue-select
which is out out scope here.
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 the vertical position of the drop down needs to be changed. But not sure this is possible without changing the vue-select
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 the vertical position of the drop down needs to be changed. But not sure this is possible without changing the vue-select
I tested this (manually). It won't help, since both elements, the trigger and the dropdown, have an outline.
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 we merge this pr and improve left things in next future?
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.
ok, i will merge it now, got enough approves
☑️ Resolves
🖼️ Screenshots
🏚️ Before
🏡 After
🏁 Checklist