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

Disable selectable text #7247

Merged

Conversation

mojganii
Copy link
Collaborator

@mojganii mojganii commented Nov 27, 2024

This PR makes the entire text in InfoHeaderView clickable and disables text selection.


This change is Reviewable

Copy link

linear bot commented Nov 27, 2024

@mojganii mojganii self-assigned this Nov 27, 2024
@mojganii mojganii added iOS Issues related to iOS enhancement labels Nov 27, 2024
rablador
rablador previously approved these changes Nov 27, 2024
Copy link
Contributor

@rablador rablador left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @mojganii)


ios/MullvadVPN/Views/InfoHeaderView.swift line 84 at r1 (raw file):

    private func addTapGestureRecognizer() {
        let tapGesture = UITapGestureRecognizer(target: self, action: #selector(handleTextViewTap))
        textView.addGestureRecognizer(tapGesture)

This change breaks accessibility and makes it impossible to open the link with voice reader on.

Copy link
Collaborator Author

@mojganii mojganii left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @buggmagnet)


ios/MullvadVPN/Views/InfoHeaderView.swift line 84 at r1 (raw file):

Previously, buggmagnet wrote…

This change breaks accessibility and makes it impossible to open the link with voice reader on.

I tested it out by voiceover and it worked, If you mean that.

Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @mojganii)


ios/MullvadVPN/Views/InfoHeaderView.swift line 84 at r1 (raw file):

Previously, mojganii wrote…

I tested it out by voiceover and it worked, If you mean that.

I meant the screen reader, sorry.

Copy link
Collaborator Author

@mojganii mojganii left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @buggmagnet)


ios/MullvadVPN/Views/InfoHeaderView.swift line 84 at r1 (raw file):

Previously, buggmagnet wrote…

I meant the screen reader, sorry.

aren't they the same?
Untitled.mp4

@mojganii mojganii force-pushed the disable-selectable-text-in-formatted-textview-ios-930 branch from 6ed0825 to 5a570fd Compare December 2, 2024 09:35
@mojganii mojganii requested a review from buggmagnet December 3, 2024 08:56
@mojganii mojganii force-pushed the disable-selectable-text-in-formatted-textview-ios-930 branch from 5a570fd to 65e2c6f Compare December 3, 2024 16:52
@mojganii mojganii requested a review from rablador December 3, 2024 16:52
Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

@buggmagnet buggmagnet merged commit d44108b into main Dec 4, 2024
10 of 11 checks passed
@buggmagnet buggmagnet deleted the disable-selectable-text-in-formatted-textview-ios-930 branch December 4, 2024 06:16
Copy link

github-actions bot commented Dec 4, 2024

🚨 End to end tests failed. Please check the failed workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement iOS Issues related to iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants