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

[fix/a11y-1334] Add Spaces text background color if Increase Contrast is on #1451

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[fix/a11y-1334] Add Spaces text background color if Increase Contrast is on #1451

wants to merge 3 commits into from
+50 −12

Conversation

felix-schwarz
Copy link
Contributor

Description

  • ThemeCollection: remove superfluous code cell
  • DriveListCell: hide subtitle label if subtitle is not set or empty
  • DriveHeaderCell: make sensitive to high contrast mode, adding a dark background if detected or changed to

Related Issue

#1334

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@felix-schwarz felix-schwarz mentioned this pull request Mar 10, 2025
Open
4 tasks
@felix-schwarz felix-schwarz requested review from hosy and jesmrec March 10, 2025 22:00
@felix-schwarz felix-schwarz self-assigned this Mar 10, 2025
@felix-schwarz felix-schwarz linked an issue Mar 10, 2025 that may be closed by this pull request
[a11y] 11.1.4.3 Contrast (minimum) #1334
Open
4 tasks
hosy
hosy requested changes Mar 14, 2025
View reviewed changes
ownCloudAppShared/Client/Collection Views/Cells/DriveHeaderCell.swift
var contrastObservation: NSObjectProtocol?

func setupHighContrastMode() {
contrastObservation = NotificationCenter.default.addObserver(forName: UIAccessibility.darkerSystemColorsStatusDidChangeNotification, object: nil, queue: .main, using: { [weak self] _ in
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove observer is missing on deinit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is no longer necessary as observers added via this method are automatically removed once the returned observation object is deallocated. As the returned contrastObservation object is released and deallocated at the time DriveHeaderCell is deinited/deallocated, the observation is automatically removed.

@hosy hosy self-requested a review March 17, 2025 09:39
@jesmrec
Copy link
Contributor

jesmrec commented Mar 17, 2025 •
edited
Loading

For text, the contrast threshold should be 4.5:1 and up

None of the existing ones, with or without the "increase Contrast" reaches that value.

Regular color High contrast color High contrast color in 47eba9a9
Size/Time info #8c8c91 (3,34) ❌ #8a8a8e (3,44) ❌ #080808 (20,03) ✅
Button labels #017aff vs #f2f2f2 (3,59) ❌ #007aff vs #f2f2f2 (3,59) ❌ #0055b2 vs #f2f2f2 (6,38) ✅
Account URL #96969a (2,95) ❌ #959599 (2,98) ❌ #2d2d2d (13,7) ✅
Shared with me #ff3b30 vs #ffdbd9 (2,77) ❌
#34c759 vs #daf5e1 (1,92) ❌		 #d80318 vs #f8d1d5 (3,82) ❌
#248a3d vs #d7eadc (3,49) ❌		 #d80318 vs #f8d1d5 (3,82) ❌
#248a3e vs #d7eadc (3,49) ❌
Shared with other #9a9a9e (2,80) ❌ #8c8c90 (3,35) ❌ #262626 (15,1) ✅

NOTE: color values are not totally exact.

Dark background for the Space name when "Increase Contrast" is checked in ✅

felix-schwarz added a commit that referenced this pull request Mar 18, 2025
@felix-schwarz
- UIColor+Extension: add method to create a color that changes to a d…
bd0b32e 
…ifferent color if high contrast mode is enabled in System Settings

- ThemeCollection:
	- add alternative 100% contrast colors for labels to further address #1451
	- fix typo in var name
	- fix swift-lint warning
@felix-schwarz
Copy link
Contributor Author

felix-schwarz commented Mar 18, 2025 •
edited
Loading

@jesmrec Thanks for the analysis. I have since made changes to increase contrast even further for those elements when High Contrast is enabled in System Settings. Please have another look.

@felix-schwarz
- ThemeCollection: remove superfluous code cell
f123dce 
- DriveListCell: hide subtitle label if subtitle is not set or empty
- DriveHeaderCell: make sensitive to high contrast mode, adding a dark background if detected or changed to
@felix-schwarz
- UIColor+Extension: add method to create a color that changes to a d…
863ead4 
…ifferent color if high contrast mode is enabled in System Settings

- ThemeCollection:
	- add alternative 100% contrast colors for labels to further address #1451
	- fix typo in var name
	- fix swift-lint warning
@felix-schwarz
- ThemeCollection: add high contrast alternative for cell tint color
47eba9a
@jesmrec
Copy link
Contributor

jesmrec commented Mar 19, 2025 •
edited
Loading

Thanks for the update @felix-schwarz. Added a column in the table above with the contrast checks in the latest commit 47eba9a9. Every text over white background is already in.

Just one exception: the green/red buttons in the "shared with me" view (Decline / Accept ). These ones are still below 4,5. Is there something else we could do?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hosy hosy hosy approved these changes

@jesmrec jesmrec Awaiting requested review from jesmrec

At least 2 approving reviews are required to merge this pull request.

Assignees

@felix-schwarz felix-schwarz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[a11y] 11.1.4.3 Contrast (minimum)
3 participants
@felix-schwarz @jesmrec @hosy