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

fix(react): adjust bx--actions-list in data table to use display grid instead of position absolute #9994

Merged
merged 16 commits into from
Nov 4, 2021

Conversation

abbeyhrt
Copy link
Contributor

Closes #9686

Updates how bx--actions-list is positioned so that whether or not it needs to scroll is sensed as expected. Because it was positioned absolutely, it didn't show the scroll bar when the items broke out of their parent container, this addresses that and adjusts the styles where needed.

Changelog

New

  • An example for a data table with lots of batch actions for testing, will be removed before merge.

Changed

  • updates the action list to display grid to right align with buttons without position: absolute needed

Removed

  • Redundant or unnecessary styles.

Testing / Reviewing

I added a story to DataTable w/ Batch Actions that has a ton of actions, shrink your screen until some aren't visible and make sure that the horizontal scroll bar appears and you can scroll to all the items.

The new story will be removed before merge.

@abbeyhrt abbeyhrt requested a review from a team as a code owner October 28, 2021 21:41
@abbeyhrt abbeyhrt requested a review from dakahn October 28, 2021 21:41
@netlify
Copy link

netlify bot commented Oct 28, 2021

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: bc51d17

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/61832c4f125efe0007786635

😎 Browse the preview: https://deploy-preview-9994--carbon-react-next.netlify.app

@netlify
Copy link

netlify bot commented Oct 28, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: bc51d17

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/61832c4f5cb85d0007440bf8

😎 Browse the preview: https://deploy-preview-9994--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Oct 28, 2021

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: bc51d17

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/61832c4fbdfabd00084a7910

😎 Browse the preview: https://deploy-preview-9994--carbon-components-react.netlify.app

Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

The scroll works good but the summary is getting cut off on the left side.

image

We could keep the summary and have the other item scroll under it OR we can just have the summary scroll with the actions.

image

Copy link
Member

@tay1orjones tay1orjones 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 to me 👍

When you hover over a button that is behind the summary, you can see that the summary isn't as tall as the buttons.
Screen Shot 2021-11-01 at 10 05 44 PM
Would be cool if the summary could be the same height as the buttons, but that might be a limitation of the layout configuration happening here. In general this is a huge improvement as-is even without that piece.

@abbeyhrt
Copy link
Contributor Author

abbeyhrt commented Nov 3, 2021

@aagonzales Good catch! I fixed the scroll issue you pointed out and adjusted the height that Taylor mentioned, would you mind doing a quick re-review?

Copy link
Member

@aagonzales aagonzales 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!

@abbeyhrt abbeyhrt enabled auto-merge (squash) November 3, 2021 19:17
@abbeyhrt abbeyhrt merged commit 963eb58 into carbon-design-system:main Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: DataTable Batch Actions cut off on smaller screens
5 participants