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

Change action sheet tint color #6096

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

mojganii
Copy link
Collaborator

@mojganii mojganii commented Apr 9, 2024

To enhance the differentiation between enabled and disabled actions, we've opted for a more distinct color that ensures better visibility. This PR implements this change.

Flowers in Chania

This change is Reviewable

@mojganii mojganii added enhancement iOS Issues related to iOS labels Apr 9, 2024
@mojganii mojganii self-assigned this Apr 9, 2024
Copy link

linear bot commented Apr 9, 2024

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 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/Coordinators/LocationCoordinator.swift line 201 at r1 (raw file):

        actionSheet.overrideUserInterfaceStyle = .dark
        actionSheet.view.tintColor = UIColor(red: 0 / 255.0, green: 150 / 255.0, blue: 255 / 255.0, alpha: 1)

Is there any reason why we don't use 0.0 instead of 255 / 255.0 ?
Which in itself is bad because we're dividing an integer with a floating point number.

@mojganii mojganii requested a review from buggmagnet April 10, 2024 09:16
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/Coordinators/LocationCoordinator.swift line 201 at r1 (raw file):

Previously, buggmagnet wrote…

Is there any reason why we don't use 0.0 instead of 255 / 255.0 ?
Which in itself is bad because we're dividing an integer with a floating point number.

I wanted to maintain consistency with other parameter formats. Instead of using '255.0' as a constant, I'll use floating values for them.

@mojganii mojganii force-pushed the change-appearance-custom-list-action-sheet-ios-600 branch from 2781267 to 1e112e3 Compare April 10, 2024 09:29
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: :shipit: complete! all files reviewed, all discussions resolved

@buggmagnet buggmagnet force-pushed the change-appearance-custom-list-action-sheet-ios-600 branch from 1e112e3 to 810ac5e Compare April 10, 2024 10:50
@buggmagnet buggmagnet merged commit b2c94cc into main Apr 10, 2024
7 checks passed
@buggmagnet buggmagnet deleted the change-appearance-custom-list-action-sheet-ios-600 branch April 10, 2024 10:50
Copy link

🚨 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.

2 participants