Skip to content
This repository was archived by the owner on Dec 11, 2019. It is now read-only.

Tabs don't resize with window #100

Closed
bridiver opened this issue Dec 26, 2015 · 18 comments
Closed

Tabs don't resize with window #100

bridiver opened this issue Dec 26, 2015 · 18 comments

Comments

@bridiver
Copy link
Collaborator

bridiver commented Dec 26, 2015

Test plan

#6900 (comment)


Steps to reproduce:

  1. Open Brave and maximize it
  2. Keep creating tabs until you reach 6
  3. Tabs will finally resize for the window

Not sure what the correct behavior should be here with a fixed number of tabs. Currently they only expand to fit the window size when there are 6 in the tab group. Maybe they should always fit the window width and get smaller until they max out at 6? @bradleyrichter ?

@bbondy
Copy link
Member

bbondy commented Dec 27, 2015

That's the expected behavior. There's only no max-width when there are a full set of tabs.
The reasoning is because the huge looking tabs would look strange. For example a single tab which spans the full window width.

@bbondy
Copy link
Member

bbondy commented Dec 27, 2015

This might feel better when we have the new-tab button jump next to the tabs when less than 6.

@bbondy
Copy link
Member

bbondy commented Dec 28, 2015

The new-tab sticking is implemented now, please take a look and let me know if this can be closed.

@bridiver
Copy link
Collaborator Author

Doesn't seem to have changed. Still seems weird to me that they stay the same size until you get 6 and then they resize to fit the full window

@bradleyrichter
Copy link
Contributor

@bridiver I agree that the auto-resize change upon 6 tabs isn't ideal but it is probably better than the safari approach in our UI case. I did have one idea to improve this which is to have empty tab spots. So in the case if a new set, you would have one real tab and 6 more empty outlines that would allow for stretching right away instead of waiting for the 6th tab. I don't know if it would help or just add visual clutter. Let's see if the sticky + button helps this issue for you.

@psimyn
Copy link
Contributor

psimyn commented Feb 14, 2016

Can alternatively set tabArea maxWidth programmatically with calc() - does mean that on narrow viewports with only one or two tabs they look very narrow though

style={{maxWidth: `calc(${100 / this.props.tabsPerTabPage}% - ${addTabButtonWidth / this.props.tabsPerTabPage}px)`}}

tabstretch

@bridiver
Copy link
Collaborator Author

@bradleyrichter and I can up with a formula to keep them from growing too large or too small, we just need to implement it. Maybe he can post the details here?

@cezaraugusto
Copy link
Contributor

I'm late here but the UI seems good for me at this moment and the conversation here hadn't been concluded. Can we close it?

@cezaraugusto cezaraugusto added the needs-info Another team member needs information from the PR/issue opener. label Jun 16, 2016
@bsclifton
Copy link
Member

I've confirmed this still is an issue and it does seem a bit weird.

STR that make it obvious:

  1. Maximize window
  2. Go to prefs; set Tabs => Tabs per tab page to 6
  3. Keep hitting the + next to tabs until you have 5 tabs
  4. Hit the + one more time- notice that all the tabs resize themselves, becoming much wider
  5. Close the 6th tab- notice all the tabs shrink back to their previous size

This problem seems to be masked because the browser (at least in my case) defaults to 10 tabs per page. That is enough tabs that it does properly fill out the window and it's not noticeable.

@bradleyrichter: are you happy with the current behavior?

@bradleyrichter
Copy link
Contributor

The new 10-tab default reduced the problem as you said @bsclifton but ideally we would implement a solution. I am game for trying @psimyn 's suggestion above because we are already doing this when reducing the window width.

@bridiver
Copy link
Collaborator Author

bridiver commented Jun 29, 2016

@bradleyrichter and I came up with a good sizing algorithm, but I don't think it ever got implemented. Do you still have the notes? Mentioned above but there was no update.

@bridiver
Copy link
Collaborator Author

bridiver commented Jun 29, 2016

I'm not sure if @psimyn will always resize correctly because it doesn't appear to leave space for an extra tab at the end when you have less than the maximum number of tabs in a page. We came up with a really clean and simple way to do it, but I'd rather not try to reconstruct it if we still have it somewhere

@bradleyrichter
Copy link
Contributor

here are those notes:

newtab_scaling_approach_desktop.pdf

@bridiver
Copy link
Collaborator Author

I know we came up with an actual algorithm for it. Is that all you could find?

@bradleyrichter
Copy link
Contributor

That was the visual explanation. I don't recall an algorithm. But I am not seeing a problem with Simon's approach based on the video snip.

@bridiver
Copy link
Collaborator Author

I guess it does look like it handles the last tab space, but there was an issue with both very small windows and very large windows. We tried a similar simple sizing algorithm and it didn't handle those cases very well.

@bridiver
Copy link
Collaborator Author

the large window is probably not as much of an issue with a default of 10 tabs

@cezaraugusto
Copy link
Contributor

ok based on previous comments I don't think we have an issue for large screens anymore, and tabArea has a max-width of 184px currently.

I referenced this issue on #6900, which addresses small screens (or when you have a lot of tabs).

If someone think it's still an issue LMK and I'll revoke auto-closing keyword from there.

bsclifton pushed a commit that referenced this issue Feb 6, 2017
@bsclifton bsclifton added this to the 0.13.3 milestone Feb 6, 2017
@luixxiul luixxiul added QA/test-plan-specified release-notes/include and removed needs-info Another team member needs information from the PR/issue opener. labels Feb 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants