Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(tabs): refactor css vars #1068

Merged
merged 3 commits into from
Jan 8, 2019

Conversation

mcoker
Copy link
Contributor

@mcoker mcoker commented Dec 5, 2018

fixes #1002

@andybraren @mattnolting can you guys peek at this and tell me if it looks OK to you? I made 2 commits. One for the initial refactor work. Another for making the tab buttons visible when the start/end arrow buttons are visible. re: the second commit, that's how I would expect the tabs to look with the directional arrows, but I'm assuming the way it was initially was on purpose?

@patternfly-build
Copy link

patternfly-build commented Dec 5, 2018

Deploy preview for pf-next ready!

Built with commit 47cd377

https://deploy-preview-1068--pf-next.netlify.com

@andybraren
Copy link
Contributor

The refactor looks good to me. 👍

My thinking behind positioning the directional buttons on top of, rather than next to, tabs was to prevent sudden jumping when the .pf-m-start and .pf-m-end classes are added/removed via JavaScript to make them appear/disappear.

I agree that it kinda looks like a mistake in our static example, but in motion I think overlaid directional buttons would provide a better UX.

@mcoker
Copy link
Contributor Author

mcoker commented Dec 6, 2018

@andybraren ah interesting, do the directional arrow appear when there is overflow content in the nav? I didn't realize they would appear dynamically. The problem with them being under the buttons is that the buttons cover the tab text. Is that OK? To make it less jarring, we could leave it the way you had it, but when the buttons appear/disappear dynamically, we apply a margin-left/right to the list that matches the button width, and transition that margin change (so the list resizes smoothly).

@andybraren
Copy link
Contributor

@mcoker Yeah, arrows only appear when there are more tabs in that particular direction, determined by JS. When the scroll region reaches one end the corresponding button should disappear according to the spec, revealing the rest of the first/last tab:

pf4-tabs-arrows

Assuming that JS can do this - detecting when the scroll container has reached one end or the other and acting accordingly - then I think that would obviate the need for the margin-left/right solution.

@mcoker
Copy link
Contributor Author

mcoker commented Dec 6, 2018

@andybraren ah gotcha, now I get it. Undid that commit.

@christiemolloy
Copy link
Member

@mcoker @andybraren I noticed that on the smallest viewport, the tabs arent hidden. Wondering if thats to do with the workspace or a bug?
screen shot 2018-12-14 at 10 29 31 am

@mcoker
Copy link
Contributor Author

mcoker commented Dec 14, 2018

@christiemolloy good catch, updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants