Bug 1145604 - Close button for tabs in table view #290
Conversation
| super.init(style: style, reuseIdentifier: reuseIdentifier) | ||
|
|
||
| self.closeTab.setImage(UIImage(named: "toolbar_stop.png"), forState: UIControlState.Normal) | ||
| self.closeTab.backgroundColor = UIColor.clearColor() |
There was a problem hiding this comment.
Is this line necessary? I thought clear was the default background color.
|
Thanks for the pull request! This looks pretty good. I'd like to fix a few of the comments like extracting the vars to UX will also probably want to change the interaction some, but we can handle that in a polish follow-up if they don't have time to give feedback right now. |
|
@thebnich Thank you for taking time reviewing it so quickly. I really appreciate it. I have made all the changes suggested by you, please take a look at it. |
sachin004
force-pushed
the
tab-close-button
branch
from
4a00797
to
60dcbe6
Compare
|
@thebnich let me know if I need to make any more changes or is this good to go? |
| @@ -113,6 +124,8 @@ private class TabCell: UITableViewCell { | |||
|
|
|||
| innerStroke.frame = background.frame | |||
|
|
|||
| closeTab.frame = CGRect(x: backgroundHolder.frame.width - TabTrayControllerUX.CloseButtonSize/2 - TabTrayControllerUX.Margin, y: TabTrayControllerUX.CloseButtonMargin, width: TabTrayControllerUX.CloseButtonSize, height: TabTrayControllerUX.CloseButtonSize) | |||
There was a problem hiding this comment.
I'm not sure I understand how you're calculating the x value. Why divide TabTrayControllerUX.CloseButtonSize by 2? And why subtract TabTrayControllerUX.Margin? The end result might look OK right now, but we should make sure the math makes sense instead of arbitrarily putting values together.
I think the following is more intuitive:
backgroundHolder.frame.width - TabTrayControllerUX.CloseButtonSize - TabTrayControllerUX.CloseButtonMargin
|
@sachin004 thanks for the updates, and sorry for the delay! Your PR got buried by all the other open PRs :) For the future, one tip that will make sure your PR gets reviewed quickly is to flip the attachment flag to "review? bnicholson" (or someone else) in Bugzilla every time you update the pull request. This adds it to the reviewer's queue so it doesn't get overlooked. |
|
Thanks, this looks great! Squashed and pushed 74c739a. |
…opened from share menu (mozilla-mobile/focus-ios#292)
Code to fix the bug https://bugzilla.mozilla.org/show_bug.cgi?id=1145604.
Added a close button at the top right of the tab