Let's make it part of the "new tab" button's hit area instead.

Comment on attachment 8522359 [details] [diff] [review] Remove dead area on the left of "new tab" button (r=mcomella) Review of attachment 8522359 [details] [diff] [review]: ----------------------------------------------------------------- I see this removing a dead space to the right of the new tab button, not the left, but to be fair, I'm not familiar enough with this issue in the first place. Can you explain a little how this fixes the issue? ::: mobile/android/base/newtablet/res/layout-large-v11/tab_strip.xml @@ +19,3 @@ > android:src="@drawable/tab_new_level" > android:contentDescription="@string/new_tab" > + android:layout_gravity="start" I never really understood `layout_gravity="start/end"` - is this case equivalent to `layout_gravity="top|left"`?

(In reply to Michael Comella (:mcomella) from comment #2) > Comment on attachment 8522359 [details] [diff] [review] > Remove dead area on the left of "new tab" button (r=mcomella) > > Review of attachment 8522359 [details] [diff] [review]: > ----------------------------------------------------------------- > > I see this removing a dead space to the right of the new tab button, not the > left, but to be fair, I'm not familiar enough with this issue in the first > place. Can you explain a little how this fixes the issue? Long story short: we're using margin to align the 'new tab' button with the menu button in the toolbar. Using margin there means we're adding a dead area on the right of the 'new tab' button. Stepping back a bit, I think a simpler/better way to fix this is to use a TouchDelegate instead. I'll post a new patch. > > ::: mobile/android/base/newtablet/res/layout-large-v11/tab_strip.xml > @@ +19,3 @@ > > android:src="@drawable/tab_new_level" > > android:contentDescription="@string/new_tab" > > + android:layout_gravity="start" > > I never really understood `layout_gravity="start/end"` - is this case > equivalent to `layout_gravity="top|left"`? start/end only deals with left/right. It's the way build RTL/LTR-agnostic UIs.

(In reply to Lucas Rocha (:lucasr) from comment #3) > Using margin there means we're adding a dead > area on the right of the 'new tab' button.

Comment on attachment 8522359 [details] [diff] [review] Remove dead area on the left of "new tab" button (r=mcomella) Review of attachment 8522359 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Lucas Rocha (:lucasr) from comment #3) > I think a simpler/better way to fix this is to use a > TouchDelegate instead. I think this sounds more complicated than the current solution.

Comment on attachment 8526682 [details] [diff] [review] Remove dead area on the right of "new tab" button (r=mcomella) Here's a much simpler/saner approach.

Comment on attachment 8526682 [details] [diff] [review] Remove dead area on the right of "new tab" button (r=mcomella) Review of attachment 8526682 [details] [diff] [review]: ----------------------------------------------------------------- Ahh, I see - this is pretty clean. I'm a little concerned that other developers might change the layout and not know why the right side of the tab strip creates new tabs so it'd be good to add a comment to the addTab button's declaration in the layout xml that mentions there is a touch delegate associated with this view.

(In reply to Michael Comella (:mcomella) from comment #8) > Comment on attachment 8526682 [details] [diff] [review] > Remove dead area on the right of "new tab" button (r=mcomella) > > Review of attachment 8526682 [details] [diff] [review]: > ----------------------------------------------------------------- > > Ahh, I see - this is pretty clean. > > I'm a little concerned that other developers might change the layout and not > know why the right side of the tab strip creates new tabs so it'd be good to > add a comment to the addTab button's declaration in the layout xml that > mentions there is a touch delegate associated with this view. Ok, done.