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

fix: High contrast mode optimization #3756

Merged
merged 1 commit into from Feb 27, 2023
Merged

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Feb 15, 2023

Add the missing right border on the app navigation (high contrast mode). Unify design with settings view where the app navigation has a border on the right on high contrast mode.

Changed the background color of the active element on high contrast mode, as the highlighting was barley visible.

Also applied the styles for the NcListItem to have a common styling across apps, as some apps, like the forms app, use list items instead of app navigation items (to allow a second title within the item).

Normal High-contrast before High-contrast now
Screenshot_20230215_025927 Screenshot_20230215_030016 Screenshot_20230215_031029

Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>
@susnux susnux added 3. to review Waiting for reviews feature: app-navigation Related to the app-navigation component design Design, UX, interface and interaction design labels Feb 15, 2023
Copy link
Contributor

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Very nice – I would even say your fix could be applied generally, not only for high contrast mode. Then the navigation would always have a slight border to the right, which looks a bit nicer.

For example, it looks especially strange on narrow screens when opening the navigation:

Currently With your patch
image image

It also looks very nice with the general style in bigger desktop view, with the background shining through:
image

What do you think @susnux? :)

Copy link

@nimishavijay nimishavijay left a comment

Choose a reason for hiding this comment

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

Agreed with Jan! This definitely makes sense here but we can also do it normally :)

@mejo- mejo- modified the milestones: 7.7.0, 7.8.0 Feb 22, 2023
@raimund-schluessler raimund-schluessler merged commit 82168c2 into master Feb 27, 2023
@raimund-schluessler raimund-schluessler deleted the fix/high-contrast branch February 27, 2023 12:24
@jancborchardt
Copy link
Contributor

@susnux mind sending another pull request which just adds a border in general? :)

@julien-nc julien-nc mentioned this pull request Feb 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 design Design, UX, interface and interaction design feature: app-navigation Related to the app-navigation component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App navigation missing right border in high-contrast mode
6 participants