Skip to content

fix(Tabs): enable tabs scroll button for small window#6784

Merged
nicolethoen merged 4 commits intopatternfly:mainfrom
DaoDaoNoCode:tabs-overflow
Jan 18, 2022
Merged

fix(Tabs): enable tabs scroll button for small window#6784
nicolethoen merged 4 commits intopatternfly:mainfrom
DaoDaoNoCode:tabs-overflow

Conversation

@DaoDaoNoCode
Copy link
Copy Markdown
Contributor

@DaoDaoNoCode DaoDaoNoCode commented Jan 13, 2022

What: Closes #5175
This issue is because there the button only scrolls left or right when the tab is totally inside the view. However, when the window is small, the tab content could even be longer than the list width, which makes it can never find the element in the view. To solve the problem, I created a new optional prop named strict and made a judgment on the element width and the container width, if the container width is smaller than the element width and the strict is not set, we treat it as part in view. Only when strict mode is set, we will not take the element width and container width into consideration, which brings more flexibility to the function.
Also, I update the bounding to make it more precise to judge whether an element is in view when the element width is not an integer. And add a debounce to the scroll function.

@DaoDaoNoCode DaoDaoNoCode requested a review from dlabrecq January 13, 2022 21:28
@patternfly-build
Copy link
Copy Markdown
Collaborator

patternfly-build commented Jan 13, 2022

Comment thread packages/react-core/src/components/Tabs/Tabs.tsx Outdated
Copy link
Copy Markdown
Member

@dlabrecq dlabrecq left a comment

Choose a reason for hiding this comment

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

Other than the Lodash issue, it LGTM

@jpuzz0
Copy link
Copy Markdown
Contributor

jpuzz0 commented Jan 14, 2022

Do we know how this strict param might impact NavList (which is also using this helper)?

Also, do we know of any use-cases where we would want strict to be "true" and controllable by developers via a prop?

Comment thread packages/react-core/src/helpers/util.ts Outdated
* @returns { boolean } True if the component is in View.
*/
export function isElementInView(container: HTMLElement, element: HTMLElement, partial: boolean) {
export function isElementInView(container: HTMLElement, element: HTMLElement, partial: boolean, strict?: boolean) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm thinking it might be better to default this value to false, so strict: boolean = false so we don't have to count on undefined being checked correctly down below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will update, thanks!

@DaoDaoNoCode
Copy link
Copy Markdown
Contributor Author

DaoDaoNoCode commented Jan 14, 2022

Do we know how this strict param might impact NavList (which is also using this helper)?

Yes, the same problem also exists on NavList, you could check this example https://www.patternfly.org/v4/components/navigation#horizontal-only-in-pageheader and set the width to very small. So this could also help solve the issue in NavList.

Also, do we know of any use-cases where we would want strict to be "true" and controllable by developers via a prop?

Currently, my answer is no. But I prefer to keep this prop. If they want to check whether an element is inside a container in the future in some other situations this could help.

@jpuzz0
Copy link
Copy Markdown
Contributor

jpuzz0 commented Jan 14, 2022

Thanks for clarifying!

@nicolethoen nicolethoen added this to the 2022.01 milestone Jan 17, 2022
@nicolethoen nicolethoen merged commit a29b430 into patternfly:main Jan 18, 2022
@patternfly-build
Copy link
Copy Markdown
Collaborator

Your changes have been released in:

  • eslint-plugin-patternfly-react@4.28.5
  • @patternfly/react-catalog-view-extension@4.40.5
  • @patternfly/react-charts@6.42.5
  • @patternfly/react-code-editor@4.30.5
  • @patternfly/react-console@4.40.5
  • @patternfly/react-core@4.189.5
  • @patternfly/react-docs@5.50.5
  • @patternfly/react-icons@4.40.5
  • @patternfly/react-inline-edit-extension@4.34.5
  • demo-app-ts@4.149.5
  • @patternfly/react-integration@4.151.5
  • @patternfly/react-log-viewer@4.34.5
  • @patternfly/react-styles@4.39.5
  • @patternfly/react-table@4.58.5
  • @patternfly/react-tokens@4.41.5
  • @patternfly/react-topology@4.36.5
  • @patternfly/react-virtualized-extension@4.36.5
  • transformer-cjs-imports@4.27.5

Thanks for your contribution! 🎉

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.

Tab overflow disabled for small windows

5 participants