Missing bottom border on Clear Private Data button

VERIFIED FIXED

Status

()

Firefox for iOS
Theme & Visual Design
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: darrin, Assigned: etienne, Mentored)

Tracking

unspecified
Other
iOS

Firefox Tracking Flags

(fxios1.3+)

Details

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
Created attachment 8686629 [details]
Screenshot

Looks like this is just a table cell vs. being a proper full-width button. Bottom border should match top border.
Mentor: dhenein@mozilla.com
tracking-fxios: ? → ---
Whiteboard: [good first bug][lang=swift]
(Assignee)

Comment 1

2 years ago
Created attachment 8691931 [details] [review]
Pull request
Assignee: nobody → etienne
Attachment #8691931 - Flags: review?(dhenein)
(Assignee)

Comment 2

2 years ago
Created attachment 8691932 [details]
Before / After Screenshot

Hey Darrin!
Don't know if you're reviewing the screenshot/the code or both, but feel free to redirect :)
Attachment #8691932 - Flags: ui-review?(dhenein)
Comment on attachment 8691931 [details] [review]
Pull request

I can take the code review part if you want :darrin
Attachment #8691931 - Flags: review?(dhenein) → review?(sleroux)
Comment on attachment 8691931 [details] [review]
Pull request

Looks good to me! The only thing is maybe renaming the header view to reflect that it's also used as a footer now.
Attachment #8691931 - Flags: review?(sleroux) → review+
Comment on attachment 8691931 [details] [review]
Pull request

Just noticed something in landscape:

https://github.com/mozilla/firefox-ios/pull/1304#issuecomment-159625961
Attachment #8691931 - Flags: review+ → review?
(Reporter)

Comment 6

2 years ago
Comment on attachment 8691932 [details]
Before / After Screenshot

This looks great! For details sake, I'm wondering why the bottom border appears to be thicker/heavier than the top border on that view?

If it's just a height/border width issue, it should be a quick fix and +.
Attachment #8691932 - Flags: ui-review?(dhenein) → ui-review+
Comment on attachment 8691931 [details] [review]
Pull request

Thanks for the quick fix! Looks like the double thickness issue is also gone that :darrin mentioned.
Attachment #8691931 - Flags: review? → review+
Landed

24dfac4dd89ffb0433730125c381afbdcad71ba3
Status: NEW → RESOLVED
Last Resolved: 2 years ago
tracking-fxios: --- → 1.3+
Resolution: --- → FIXED
Whiteboard: [good first bug][lang=swift] → [needsuplift]
(Assignee)

Comment 9

2 years ago
Cool, fun project to contribute too :) Thanks!
Whiteboard: [needsuplift]
Reproduced the issue on version 1.2 build 1206 using Iphone 6 (IOS 9.1). Verified as fixed on version 1.3 build 1308.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.