Skip to content

Conversation

@zeroedin
Copy link
Contributor

@zeroedin zeroedin commented Apr 26, 2023

What I did

  1. Refactored scroll calculation for scrollLeft and scrollRight in overflow controller
  2. Added smooth scroll behavior
  3. Refactored showScrollButtons calculation to include scroll button width in hiding and showing the buttons.

Testing Instructions

  1. View Deploy preview

Notes to Reviewers

  1. Ensure that this works across all browsers and screen sizes. Please use Browserstack and try a variety of devices

@zeroedin zeroedin added the bug label Apr 26, 2023
@zeroedin zeroedin self-assigned this Apr 26, 2023
@changeset-bot
Copy link

changeset-bot bot commented Apr 26, 2023

🦋 Changeset detected

Latest commit: 3c310e3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@patternfly/pfe-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the work in progress POC / Not ready for review label Apr 26, 2023
@zeroedin zeroedin changed the title fix(tabs): scroll on small screens fix(core): overflow controller scroll on small screens Apr 26, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 26, 2023

Deploy Preview for patternfly-elements ready!

Name Link
🔨 Latest commit 8607d19
😎 Deploy Preview https://deploy-preview-2486--patternfly-elements.netlify.app/

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions github-actions bot added the AT passed Automated testing has passed label Apr 26, 2023
@zeroedin zeroedin requested review from heyMP and nikkimk April 26, 2023 14:41
Copy link
Contributor

@nikkimk nikkimk 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, except for that tabs breaks in Safari versions older than 14.

@nikkimk nikkimk self-requested a review April 26, 2023 17:10
Copy link
Contributor

@nikkimk nikkimk left a comment

Choose a reason for hiding this comment

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

TL;DR: LGTM

So the issue I found on Safari is also an issue on https://patternflyelements.org/components/tabs/demo but not on the DP for #2845 .

This tells me that this PR doesn't break Safari and that the other PR should fix it.

@zeroedin zeroedin marked this pull request as ready for review April 26, 2023 17:56
Copy link
Contributor

@heyMP heyMP left a comment

Choose a reason for hiding this comment

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

Smooth! 👍

@zeroedin zeroedin merged commit 28f26cb into main Apr 26, 2023
@zeroedin zeroedin deleted the fix/tabs/scroll-on-small-screen branch April 26, 2023 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AT passed Automated testing has passed bug work in progress POC / Not ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants