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.

Sign up for GitHub

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

Jump to bottom

adjust color of focus-visible to be better visible #40783

Merged
merged 1 commit into from Oct 9, 2023
Merged

adjust color of focus-visible to be better visible #40783

merged 1 commit into from Oct 9, 2023

Conversation

szaimen
Copy link
Contributor

@szaimen szaimen commented Oct 5, 2023 •
edited

Close #40710

Before

Aufzeichnung.2023-10-05.122622.mp4

After

Aufzeichnung.2023-10-05.122508.mp4

@szaimen szaimen added bug 2. developing Work in progress labels Oct 5, 2023
@szaimen szaimen added this to the Nextcloud 28 milestone Oct 5, 2023
@szaimen szaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 5, 2023
@szaimen szaimen marked this pull request as ready for review October 5, 2023 10:29
@szaimen szaimen added enhancement design Design, UI, UX, etc. accessibility and removed bug labels Oct 5, 2023
JuliaKirschenheuter
JuliaKirschenheuter reviewed Oct 5, 2023
View reviewed changes
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

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

should we use a same styles as from nextcloud-libraries/nextcloud-vue#4193 like (?)

outline: 2px solid var(--color-main-text) !important; box-shadow: 0 0 0 4px var(--color-main-background) !important;

@szaimen
Copy link
Contributor Author

szaimen commented Oct 5, 2023

should we use a same styles as from nextcloud-libraries/nextcloud-vue#4193 like (?)

outline: 2px solid var(--color-main-text) !important; box-shadow: 0 0 0 4px var(--color-main-background) !important;

I don't think this is needed here honestly :)

JuliaKirschenheuter
JuliaKirschenheuter requested changes Oct 5, 2023
View reviewed changes
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter 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 then we would be on a same state like everywhere else

core/css/apps.scss Outdated Show resolved Hide resolved
@JuliaKirschenheuter
Copy link
Contributor

@jancborchardt what would you say?

@jancborchardt
Copy link
Member

I'd agree with you @JuliaKirschenheuter, we should have it consistently everywhere with the black + white border as this catches all cases and is very visible.

@szaimen szaimen force-pushed the enh/40710/adjust-focus-visible branch from aca8e30 to 7a40396 Compare October 6, 2023 13:59
JuliaKirschenheuter
JuliaKirschenheuter approved these changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

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

could you please build everything?

@szaimen
Copy link
Contributor Author

szaimen commented Oct 9, 2023

could you please build everything?

Yes, will do a rebase and build and merge :)

@szaimen szaimen enabled auto-merge October 9, 2023 14:41
@szaimen szaimen disabled auto-merge October 9, 2023 14:44
@szaimen
Copy link
Contributor Author

szaimen commented Oct 9, 2023

/rebase

@szaimen szaimen force-pushed the enh/40710/adjust-focus-visible branch from 7a40396 to bf6db89 Compare October 9, 2023 14:48
@szaimen
Copy link
Contributor Author

szaimen commented Oct 9, 2023

/compile amend /

@szaimen szaimen enabled auto-merge October 9, 2023 15:20
@szaimen @nextcloud-command
adjust color of focus-visible to be better visible
10c0c30 
Signed-off-by: Simon L <szaimen@e.mail.de>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@szaimen szaimen merged commit 5acf6c6 into master Oct 9, 2023
39 checks passed
@szaimen szaimen deleted the enh/40710/adjust-focus-visible branch October 9, 2023 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@susnux susnux susnux approved these changes

@JuliaKirschenheuter JuliaKirschenheuter JuliaKirschenheuter approved these changes

@ShGKme ShGKme Awaiting requested review from ShGKme

@Pytal Pytal Awaiting requested review from Pytal

@jancborchardt jancborchardt Awaiting requested review from jancborchardt

Assignees
No one assigned
Labels
3. to review Waiting for reviews accessibility design Design, UI, UX, etc. enhancement
Projects
None yet
Milestone
Nextcloud 28
Development

Successfully merging this pull request may close these issues.

[BITV]: Focused item on settings nav is not recognizable.
4 participants
@szaimen @JuliaKirschenheuter @jancborchardt @susnux