Skip to content

feat: add columns resizable options #2935

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

Conversation

mmarchett
Copy link
Contributor

@mmarchett mmarchett commented Jul 21, 2021

Resolves: B2BSOLUT-5767

Fixes #2936

Additional description

Added the functionality to modify the width of the columns in the data-table component.

CONTRIBUTOR checklist (do not remove)

Please complete for every pull request

  • First-time contributors should sign the Contributor License Agreement. It's a fancy way of saying that you are giving away your contribution to this project. If you haven't before, wait a few minutes and a bot will comment on this pull request with instructions.
  • npm run lint:fix has been run and linting passes.
  • Mocha, Jest (Storyshots), and components/component-docs.json CI tests pass (npm test).
  • Tests have been added for new props to prevent regressions in the future. See readme.
  • Review the appropriate Storybook stories. Open http://localhost:9001/.
  • Review tests are passing in the browser. Open http://localhost:8001/.
  • Review markup conforms to SLDS by looking at DOM snapshot strings.

REVIEWER checklist (do not remove)

  • CircleCI tests pass. This includes linting, Mocha, Jest, Storyshots, and components/component-docs.json tests.
  • Tests have been added for new props to prevent regressions in the future. See readme.
  • Review the appropriate Storybook stories. Open http://localhost:9001/.
  • The Accessibility panel of each Storybook story has 0 violations (aXe). Open http://localhost:9001/.
  • Review tests are passing in the browser. Open http://localhost:8001/.
  • Review markup conforms to SLDS by looking at DOM snapshot strings.
Required only if there are markup / UX changes
  • Add year-first date and commit SHA to last-slds-markup-review in package.json and push.
  • Request a review of the deployed Heroku app by the Salesforce UX Accessibility Team.
  • Add year-first review date, and commit SHA, last-accessibility-review, to package.json and push.
  • While the contributor's branch is checked out, run npm run local-update within locally cloned site repo to confirm the site will function correctly at the next release.

@welcome
Copy link

welcome bot commented Jul 21, 2021

Thanks for opening this pull request! 💯

This is a community-driven project, and we can't do it without your participation. Please check out our contributing guidelines and review the Contributor Checklist if you haven't already, to make sure everything is squared away. CircleCI will take about 10 minutes to run through the same items that are on the Contributor checklist with a pass/fail check below. Please fix any issues that cause CircleCI to fail or ask for clarification--we try, but sometimes the errors can be unclear.
A maintainer will try to respond within 7 days. If you haven’t heard anything by then, please bump this thread. To ensure codebase quality, large code line changes may take more than 2 weeks to review, but may take longer depending on the number of pull requests in the queue. Feel free to ask for a status update at any time--you won't be bothering anyone.
Once feedback has been given, please reply to the feedback giver once the feedback on been addressed, so that they can continue the review.
If you need a release while you are waiting for a code review, you can publish a built tag to your own fork. See directions in the release README.

@salesforce-cla
Copy link

Thanks for the contribution! Before we can merge this, we need @mmarchett to sign the Salesforce.com Contributor License Agreement.

@interactivellama interactivellama self-requested a review July 21, 2021 16:40
@interactivellama
Copy link
Contributor

interactivellama commented Jul 21, 2021

I've started an issue and spec for this feature.
Please add the following two bullet points to the PR #2936 to allow the feature to be accessible.

@interactivellama
Copy link
Contributor

interactivellama commented Jul 24, 2021

All the column cells have a tab stop now which is great! That should only be the case in action/edit mode however. The DataTable should only have one tab stop for the whole table when in Navigation mode. Now all cells have a tab index when in Navigation mode.

The tab events within a cell are broken now. Tab key now takes the user to the next column even if there are multiple tabbable items within a cell. It should only tab to the next column after tabbing through all tabbable items in a cell. See Opportunity Name column on http://localhost:9001/?path=/story/sldsdatatable--interactive-elements when in edit mode.

Copy link
Contributor

@interactivellama interactivellama left a comment

Choose a reason for hiding this comment

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

I did a quick first pass of the code and left a few comments. This is looking great! We are getting really close.

Due to the "double header" way that the fixed layout feature is implemented, we will want to set the width of the "second header table column" to the same width on initial resize. It works when the table is scrolled due to events which is great, but isn't sync'd initially when columns are resized when fixedLayout/fixedHeader is enabled. See below:

Screen Shot 2021-07-29 at 7 39 10 AM

Does your usage/UX need non-scrollable tables to be column resizable? After reviewing the examples, I believe it would be best to limit resizing to only fixedLayout/fixedHeader. This isn't something I realized early on. Does that work for your use case? @mmarchett This PR will make the usage more predictable and limit what we need to support/debug in the future! mmarchett#2

For instance, https://developer.salesforce.com/docs/component-library/bundle/lightning-datatable/example doesn't even allow a non-scrollable/basic table.

interactivellama and others added 2 commits July 29, 2021 14:33
* Make resizable require keyboardNavigation

* Remove non fixed header example

* Rename variables in examples

* Limit resizable use to fixed layout and header
…ent on table reload. Add unique id for header-cell. Add documentation for resize feature.
@mmarchett
Copy link
Contributor Author

I did a quick first pass of the code and left a few comments. This is looking great! We are getting really close.

Due to the "double header" way that the fixed layout feature is implemented, we will want to set the width of the "second header table column" to the same width on initial resize. It works when the table is scrolled due to events which is great, but isn't sync'd initially when columns are resized when fixedLayout/fixedHeader is enabled. See below:

Screen Shot 2021-07-29 at 7 39 10 AM

Does your usage/UX need non-scrollable tables to be column resizable? After reviewing the examples, I believe it would be best to limit resizing to only fixedLayout/fixedHeader. This isn't something I realized early on. Does that work for your use case? @mmarchett This PR will make the usage more predictable and limit what we need to support/debug in the future! mmarchett#2

For instance, https://developer.salesforce.com/docs/component-library/bundle/lightning-datatable/example doesn't even allow a non-scrollable/basic table.

Thanks for the feedback, as you advised, now the resize functionality would be available in conjunction with fixedLayout only, as specified at https://react.lightningdesignsystem.com/components/data-tables/

Copy link
Contributor

@interactivellama interactivellama left a comment

Choose a reason for hiding this comment

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

This is looking great! UX is good. I left one request for tests.

Copy link
Contributor

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

@interactivellama interactivellama merged commit c59f619 into salesforce:master Aug 4, 2021
@welcome
Copy link

welcome bot commented Aug 4, 2021

Congrats on merging your first pull request to Design System React! 🎉
If you have a moment, please fill out this five question survey, we would appreciate it.
On behalf of Salesforce's customers, partners, product specialists and employees, we would like offer sincere thanks and appreciation for helping make our user experience better. We look forward to working with you more in the future.
This definitely calls for a high five! Alt High Five

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.

DataTable: Add resizable columns
2 participants