Correctly set insets for primary button #7061
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pinkisemils
force-pushed
the
center-the-text-of-the-connect-button-relative-to-the-whole-ios-906
branch
from
433a926
to
5ca4db7
Compare
pinkisemils
force-pushed
the
center-the-text-of-the-connect-button-relative-to-the-whole-ios-906
branch
from
5ca4db7
to
91e4df9
Compare
rablador
requested changes
Oct 24, 2024
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pinkisemils)
ios/MullvadVPN/View controllers/Tunnel/DisconnectSplitButton.swift line 45 at r2 (raw file):
} primaryButton.configuration?.contentInsets.leading += UIMetrics.DisconnectSplitButton.secondaryButton.width
Use the secondaryButtonSize var declared above instead.
pinkisemils
force-pushed
the
center-the-text-of-the-connect-button-relative-to-the-whole-ios-906
branch
from
91e4df9
to
b104096
Compare
rablador
approved these changes
Oct 24, 2024
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pinkisemils)
The size of the secondary button wasn't being used to adjust the insets of the disconnect button. I've gone and done did make it do it now. Feel free to reject or suggest a better solution.
pinkisemils
force-pushed
the
center-the-text-of-the-connect-button-relative-to-the-whole-ios-906
branch
from
b104096
to
7f09c7e
Compare
pinkisemils
deleted the
center-the-text-of-the-connect-button-relative-to-the-whole-ios-906
branch
|
🚨 End to end tests failed. Please check the failed workflow run. |
|
🚨 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The size of the secondary button wasn't being used to adjust the insets of the disconnect button. I've gone and done did make it do it now. Feel free to reject or suggest a better solution.
Since the inset rectangle is RTL aware, I presume this change would work there too? Or do we not display the reconnect button on the left in RTL locales?
This change is