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

Enable VoiceOver in timeline menu #1907

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

alfogrillo
Copy link
Contributor

@alfogrillo alfogrillo commented Oct 16, 2023

Enabled the voice over on the timeline menu to align to EX Android (more context is here)

@alfogrillo alfogrillo requested a review from a team as a code owner October 16, 2023 10:53
@alfogrillo alfogrillo requested review from pixlwave and removed request for a team October 16, 2023 10:53
@github-actions
Copy link

github-actions bot commented Oct 16, 2023

Warnings
⚠️ Please add a changelog.
⚠️ Some of the commits are missing ticket numbers. Please consider squashing all commits that don't have a tracking number.
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 Danger Swift against a0f042d

@alfogrillo alfogrillo self-assigned this Oct 16, 2023
@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (e42977a) 70.91% compared to head (50225e6) 70.91%.

❗ Current head 50225e6 differs from pull request most recent head a0f042d. Consider uploading reports for the commit a0f042d to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1907      +/-   ##
===========================================
- Coverage    70.91%   70.91%   -0.01%     
===========================================
  Files          464      464              
  Lines        31468    31467       -1     
  Branches     15355    15355              
===========================================
- Hits         22316    22315       -1     
  Misses        8588     8588              
  Partials       564      564              
Flag Coverage Δ
unittests 54.81% <ø> (-0.17%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ces/Screens/RoomScreen/View/TimelineItemMenu.swift 76.33% <ø> (-0.11%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pixlwave
Copy link
Member

Ohhhh interesting. So whilst screen was completely broken (🙈), I'm not sure these changes end up helping the user that much as you also can't access this sheet via VoiceOver. If it's within scope for you, we should probably be adding support for all the actions in the rotor. I don't think it would end up being too different from how we build the mac context menu in terms of how it fits in.

Related issue: #660

Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

Whoops I clicked the wrong one before.

@alfogrillo alfogrillo force-pushed the alfogrillo/timeline-menu-voiceover branch from 50225e6 to a0f042d Compare October 16, 2023 13:37
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@alfogrillo
Copy link
Contributor Author

Ohhhh interesting. So whilst screen was completely broken (🙈), I'm not sure these changes end up helping the user that much as you also can't access this sheet via VoiceOver. If it's within scope for you, we should probably be adding support for all the actions in the rotor. I don't think it would end up being too different from how we build the mac context menu in terms of how it fits in.

Related issue: #660

I'm not sure these changes end up helping the user that much as you also can't access this sheet via VoiceOver.

I think we technically can (I did it), but the trigger (the long press) will be still difficult to action for blind people. 😕

If it's within scope for you, we should probably be adding support for all the actions in the rotor.

Nope, I think it's out of scope. I wanted at least to have the VoiceOver reading the options in the menu like EXA is doing.

@alfogrillo alfogrillo enabled auto-merge (squash) October 16, 2023 13:41
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@alfogrillo alfogrillo merged commit 6898419 into develop Oct 16, 2023
@alfogrillo alfogrillo deleted the alfogrillo/timeline-menu-voiceover branch October 16, 2023 14:00
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.

2 participants