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

Accessibility - Teacher - Calendar - Font size on Edit Event screen #3061

Closed

Conversation

ndrsszsz
Copy link
Contributor

affects: Student, Teacher
release note: None
test plan: No text should get cut off on EditCalendarEventScreen when font size is increased to the maximum.

I'm not quite pleased with the solution so any suggestion is appreciated.

Before:

Simulator.Screen.Recording.-.iPhone.SE.3rd.generation.-.2025-01-14.at.12.42.53.mp4

After:

Simulator.Screen.Recording.-.iPhone.SE.3rd.generation.-.2025-01-14.at.12.39.56.mp4

Checklist

  • Follow-up e2e test ticket created
  • A11y checked
  • Tested on phone
  • Tested on tablet
  • Tested in dark mode
  • Tested in light mode
  • Approve from product

affects: Student, Teacher
release note: None
test plan: No text should get cut off on EditCalendarEventScreen when font size is increased to the maximum.
@inst-danger
Copy link
Contributor

inst-danger commented Jan 14, 2025

Teacher Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Jan 14, 2025

Student Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Jan 14, 2025

Fails
🚫 Build failed, skipping coverage check
Warnings
⚠️ This pull request will not generate a release note.

Affected Apps: Student, Teacher

MBL-18306

❌ XCTest failed: CoreTests/WKHTTPCookieStoreExtensionsTests/testDeleteAllCookies
Asynchronous wait failed: Exceeded timeout of 0.1 seconds, with unfulfilled expectations: "Publisher finished".

Generated by 🚫 dangerJS against aed0fe8

Copy link
Contributor

@suhaibabsi-inst suhaibabsi-inst left a comment

Choose a reason for hiding this comment

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

Code +1

Copy link
Contributor

@suhaibabsi-inst suhaibabsi-inst left a comment

Choose a reason for hiding this comment

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

QA +1


if date != nil {
datePicker
if dynamicTypeSize > .accessibility3 {
Copy link
Contributor

@rh12 rh12 Jan 24, 2025

Choose a reason for hiding this comment

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

This limit seems arbitrary to me, but not sure whether is there a proper way to check when would we extend the screen size.
What happens when we have the clear button?

Not sure this works here, but may worth to look into ViewThatFits.

If that fails, what do you think about another way, to add a geometry: GeometryProxy to DatePickerCell and add .frame(maxWidth: geometry.size.width) in there, in place of the .lineLimit(0).
It's less pretty but maybe less hacky as well. The label may disappear in certain conditions, so this is also not a perfect solution.
We could discuss it briefly with the others on Monday.

Please update the other screens which uses this cell as well (EditCalendarToDoScreen, EditCustomFrequencyScreen)

Copy link
Contributor

Choose a reason for hiding this comment

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

We should definitely check ViewThatFits since that's the way Apple proposes for such use cases.

Copy link
Contributor

@vargaat vargaat left a comment

Choose a reason for hiding this comment

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

There's a sweet spot where the date is cropped in a single line. If I increase the font size it will break into two lines but the date text will still start out of the screen. I guess the date text itself widens the stack out of screen and the label just aligns to it.

@vargaat vargaat closed this Jan 28, 2025
@vargaat vargaat reopened this Jan 28, 2025
@ndrsszsz ndrsszsz closed this Jan 28, 2025
@ndrsszsz ndrsszsz deleted the bugfix/MBL-18306-a11y-teacher-calendar-event-font-size branch January 28, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants