Skip to content

Fix Tabs when container has a fixed height #6239

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 20, 2022
Merged

Fix Tabs when container has a fixed height #6239

merged 3 commits into from
Jul 20, 2022

Conversation

jcfilben
Copy link
Collaborator

What does this PR do?

Fixes and issue where the tabs header container was getting squished when Tabs is contained with a fixed height.
This issue shows up well when using the hpe theme because we have a bottom border on the tabs header container.
Screen Shot 2022-07-19 at 2 56 37 PM

I solved this issue by adding flex={false} to the box around the tabs header content.

While working on this I noticed a separate issue where if justify is set to anything other than 'start', scrolling left through the tabs isn't working well. This can be sen in the Tabs/Responsive story by changing justify to 'center'. To resolve this I set justify to start when the tabs are overflowing.

Where should the reviewer start?

What testing has been done on this PR?

Tested with the following code:

<Box pad="small" height="medium">
      <Tabs alignControls="start">
        <Tab title="Tab 1">
          <Heading>Title</Heading>

          <Paragraph>
            Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam
            porttitor ipsum a ligula tincidunt, tempor posuere massa gravida.
            Morbi at magna lacus. Vivamus eu consequat lectus. Quisque a aliquam
            risus. Ut vel dignissim nunc. Etiam rhoncus pellentesque porttitor.
            Sed nec gravida nisl. Cras justo ex, condimentum at dolor vel,
            lacinia efficitur dui. Nullam pellentesque metus nec felis gravida,
            vel suscipit sem condimentum. Ut hendrerit varius enim. Donec congue
            vestibulum neque accumsan fermentum. Nulla facilisi. Praesent ornare
            id magna vitae malesuada. Aenean nulla enim, pretium ac est vel,
            tristique condimentum ex. Phasellus ante enim, pellentesque ut arcu
            sit amet, semper sodales dolor.
          </Paragraph>
          <Paragraph>
            Proin sollicitudin dui eget diam mollis mattis eget id magna.
            Pellentesque tristique aliquet leo, a euismod velit molestie non.
            Donec lacinia, lectus quis mollis pretium, turpis erat sollicitudin
            mauris, vitae luctus velit lacus eu metus. Nam pulvinar mauris vitae
            massa faucibus, id semper sapien facilisis. Integer a congue ex.
            Vestibulum dapibus est fermentum felis hendrerit vulputate. Fusce
            libero odio, posuere sit amet mi vitae, cursus aliquam nulla. Nunc
            ornare augue vitae mauris ultricies gravida. In quis enim at nisi
            semper ultrices. Nunc posuere sed libero in facilisis.
          </Paragraph>
          <Paragraph>
            Etiam sit amet ligula nunc. Pellentesque pulvinar nisl ornare dui
            molestie, sed faucibus elit rhoncus. In nec pellentesque enim, eget
            ultricies tortor. Phasellus tincidunt ultrices tortor vitae
            malesuada. Proin rhoncus maximus mauris, a commodo tortor blandit
            quis. Phasellus tincidunt luctus dapibus. Sed in imperdiet enim.
            Vivamus varius ut dolor non scelerisque. Praesent volutpat ligula
            sodales augue auctor tempor. Quisque eleifend eleifend turpis nec
            egestas. Nunc dapibus semper commodo. Sed volutpat arcu eleifend
            ipsum volutpat, eu sagittis leo hendrerit. Nulla consequat justo non
            erat finibus cursus. Integer porttitor ante in ultrices semper.
          </Paragraph>
          <Paragraph>
            Praesent vel tellus sagittis, fringilla arcu vel, sodales turpis.
            Nam id accumsan nibh. Pellentesque eget diam eleifend, ultrices diam
            ac, suscipit ipsum. Curabitur euismod sodales sem vel ultricies.
            Proin euismod ipsum eros, ut efficitur felis pharetra finibus.
            Interdum et malesuada fames ac ante ipsum primis in faucibus. Ut
            tincidunt posuere augue at porttitor. Sed venenatis iaculis est, sit
            amet interdum magna varius ut. In euismod scelerisque velit, vel
            aliquet enim. Etiam lacinia justo nec nibh aliquam, et pulvinar eros
            luctus. Fusce quis nisi eu tellus imperdiet bibendum ac id nulla.
            Proin ut tincidunt libero. Phasellus sed velit eu risus hendrerit
            suscipit at eget felis. Class aptent taciti sociosqu ad litora
            torquent per conubia nostra, per inceptos himenaeos. Phasellus at
            blandit est. Phasellus fringilla nibh ac est scelerisque, id
            faucibus enim interdum.
          </Paragraph>
        </Tab>
        <Tab title="Tab 2">
          <Box margin="small" pad="large" align="center" background="brand" />
        </Tab>
      </Tabs>
    </Box>

How should this be manually tested?

Do Jest tests follow these best practices?

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • userEvent is used in place of fireEvent.
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

Closes #6235

Screenshots (if appropriate)

Do the grommet docs need to be updated?

No

Should this PR be mentioned in the release notes?

Yes
Fixed an issue with the Tab header when Tabs are contained in a fixed height container.
Fixed an issue with responsive Tabs not showing the first tab when justify is not set to 'start'.

Is this change backwards compatible or is it a breaking change?

backwards compatible

@jcfilben jcfilben requested a review from britt6612 July 19, 2022 21:57
britt6612
britt6612 approved these changes Jul 20, 2022
View reviewed changes
Copy link
Collaborator

@britt6612 britt6612 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good nice catch on the justify

@ericsoderberghp ericsoderberghp merged commit b62089b into grommet:master Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@britt6612 britt6612 britt6612 approved these changes

+1 more reviewer

@ericsoderberghp ericsoderberghp ericsoderberghp approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tabs Underlines are not sticky when tab are used in a section which has fixed height
3 participants
@jcfilben @ericsoderberghp @britt6612