-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add FXIOS-8946 - Show lock icon when url is displayed #20265
Add FXIOS-8946 - Show lock icon when url is displayed #20265
Conversation
@@ -21,6 +21,7 @@ public struct StandardImageIdentifiers { | |||
public static let bookmarkBadgeFillBlue50 = "bookmarkBadgeFillMediumBlue50" | |||
public static let crossCircleFill = "crossCircleFillMedium" | |||
public static let cross = "crossMedium" | |||
public static let lock = "lockMedium" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you add the lock icon name here you should also add the image to Client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already there.
let shouldAdjustForNonEmpty = !isURLTextFieldEmpty && !isTextFieldFocused | ||
|
||
if shouldAdjustForOverflow { | ||
updateURLTextFieldLeadingConstraint(equalTo: iconContainerStackView.leadingAnchor, constant: iconContainerStackView.frame.width * 0.5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the magic number of 0.5
coming from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's for setting the leading constraint of the urlTextField to the middle of iconContainerStackView, in order to hide de elipsis. Equivalent would be iconContainerStackView.frame.width / 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that work when the user is using a tiny or huge font size as well??
Client.app: Coverage: 31.06
Generated by 馃毇 Danger Swift against f506aef |
This pull request has conflicts when rebasing. Could you fix it @PARAIPAN9? 馃檹 |
馃摐 Tickets
Jira ticket
Github issue
馃挕 Description
urlTextField
is not in editing mode.馃摑 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120
)