- Notifications
You must be signed in to change notification settings - Fork433
Add Tree Grid Component#2181
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@aswinshenoy please see if there's a way to solve the rendering issues. I think a containing wrapper with a set height might help fix the extending lines, but I'm unsure. Let us know if you need help!
…react into tree-grid# Conflicts:#components/storybook-stories.js
| import TreeGrid from '~/components/tree-grid'; | ||
| import IconSettings from '~/components/icon-settings'; | ||
| const data = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If there were no nested rows, then a dev would use a DataTable. I don't think we need this example and base should be nested.
| import TreeGrid from '~/components/tree-grid'; | ||
| import IconSettings from '~/components/icon-settings'; | ||
| const data = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The columns should be child components. SeeDataTable component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Here is the Lightning component data for instance.
({ // eslint-disable-line init: function (cmp) { var columns = [ { type: 'text', fieldName: 'accountName', label: 'Account Name', initialWidth: 300 }, { type: 'number', fieldName: 'employees', label: 'Employees' }, { type: 'phone', fieldName: 'phone', label: 'Phone Number' }, { type: 'url', fieldName: 'accountOwner', label: 'Account Owner', typeAttributes: { label: { fieldName: 'accountOwnerName' } } }, { type: 'text', fieldName: 'billingCity', label: 'Billing City' } ]; cmp.set('v.gridColumns', columns); // data var nestedData = [ { "name": "123555", "accountName": "Rewis Inc", "employees": 3100, "phone": "837-555-1212", "accountOwner": "http://example.com/jane-doe", "accountOwnerName": "Jane Doe", "billingCity": "Phoeniz, AZ" }, { "name": "123556", "accountName": "Acme Corporation", "employees": 10000, "phone": "837-555-1212", "accountOwner": "http://example.com/john-doe", "accountOwnerName": "John Doe", "billingCity": "San Francisco, CA", "_children": [ { "name": "123556-A", "accountName": "Acme Corporation (Bay Area)", "employees": 3000, "phone": "837-555-1212", "accountOwner": "http://example.com/john-doe", "accountOwnerName": "John Doe", "billingCity": "New York, NY", "_children": [ { "name": "123556-A-A", "accountName": "Acme Corporation (Oakland)", "employees": 745, "phone": "837-555-1212", "accountOwner": "http://example.com/john-doe", "accountOwnerName": "John Doe", "billingCity": "New York, NY" }, { "name": "123556-A-B", "accountName": "Acme Corporation (San Francisco)", "employees": 578, "phone": "837-555-1212", "accountOwner": "http://example.com/jane-doe", "accountOwnerName": "Jane Doe", "billingCity": "Los Angeles, CA" } ] }, { "name": "123556-B", "accountName": "Acme Corporation (East)", "employees": 430, "phone": "837-555-1212", "accountOwner": "http://example.com/john-doe", "accountOwnerName": "John Doe", "billingCity": "San Francisco, CA", "_children": [ { "name": "123556-B-A", "accountName": "Acme Corporation (NY)", "employees": 1210, "phone": "837-555-1212", "accountOwner": "http://example.com/jane-doe", "accountOwnerName": "Jane Doe", "billingCity": "New York, NY" }, { "name": "123556-B-B", "accountName": "Acme Corporation (VA)", "employees": 410, "phone": "837-555-1212", "accountOwner": "http://example.com/john-doe", "accountOwnerName": "John Doe", "billingCity": "New York, NY", "_children": [ { "name": "123556-B-B-A", "accountName": "Allied Technologies", "employees": 390, "phone": "837-555-1212", "accountOwner": "http://example.com/jane-doe", "accountOwnerName": "Jane Doe", "billingCity": "Los Angeles, CA", "_children": [ { "name": "123556-B-B-A-A", "accountName": "Allied Technologies (UV)", "employees": 270, "phone": "837-555-1212", "accountOwner": "http://example.com/john-doe", "accountOwnerName": "John Doe", "billingCity": "San Francisco, CA" } ] } ] } ] } ] }, { "name": "123557", "accountName": "Rhode Enterprises", "employees": 6000, "phone": "837-555-1212", "accountOwner": "http://example.com/john-doe", "accountOwnerName": "John Doe", "billingCity": "New York, NY", "_children": [ { "name": "123557-A", "accountName": "Rhode Enterprises (UCA)", "employees": 2540, "phone": "837-555-1212", "accountOwner": "http://example.com/john-doe", "accountOwnerName": "John Doe", "billingCity": "New York, NY" } ] }, { "name": "123558", "accountName": "Tech Labs", "employees": 1856, "phone": "837-555-1212", "accountOwner": "http://example.com/john-doe", "accountOwnerName": "John Doe", "billingCity": "New York, NY", "_children": [ { "name": "123558-A", "accountName": "Opportunity Resources Inc", "employees": 1934, "phone": "837-555-1212", "accountOwner": "http://example.com/john-doe", "accountOwnerName": "John Doe", "billingCity": "Los Angeles, CA" } ] } ]; cmp.set('v.gridData', nestedData); var expandedRows = ["123556", "123556-A", "123556-B", "123556-B-B", "123557"]; cmp.set('v.gridExpandedRows', expandedRows); }}); // eslint-disable-lineThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The data should be very similar. I would change_children tonodes align withTree component. Please review Tree datahttps://github.com/salesforce/design-system-react/blob/master/utilities/sample-data/tree/nodes-base.jsx#L2
| }, | ||
| { | ||
| id: '2', | ||
| childOpen: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
TheTree keys are:
* expanded: string, * id: string, * label: string or node, * selected: string, * type: string, * nodes: array| }, | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| label: 'Jane Doe', | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| href: '#".js-comment-body"> Contributor
|
| /* Copyright (c) 2015-present, salesforce.com, inc. All rights reserved */ | ||
| /* Licensed under BSD 3-Clause - see LICENSE.txt or git.io/sfdc-license */ | ||
| // Implements the [Welcome Mat design pattern](https://lightningdesignsystem.com/components/welcome-mat/) in React. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Typo
components/tree-grid/index.jsx Outdated
| */ | ||
| id: PropTypes.string, | ||
| labels: PropTypes.shape({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
These aren't visible, so they go in assistiveText prop. Please see:
https://github.com/salesforce/design-system-react/blob/master/components/data-table/index.jsx#L55
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is still outstanding. ^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
oh, sorry, it's duplicate now. Please removechooseRow,selectAll.
components/tree-grid/index.jsx Outdated
| 'slds-table_bordered', | ||
| 'slds-table_edit', | ||
| 'slds-table_fixed-layout', | ||
| 'slds-table_resizable-cols', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Unless you are implementing resizable, this might need to be removed. I'd recommend not implementing it. It's not in the current Data either. Column width on the other hand is needed.
components/tree-grid/private/row.jsx Outdated
| ) | ||
| )} | ||
| <td role="gridcell" style={{ width: '3.25rem' }}> | ||
| <Button |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This component should accept aTreeRowActions component as a child.
interactivellama commentedAug 7, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Typically, this component is going to have a fixed header, but if you want to descope that for now I'm OK with that. This comments means this is OK (no scrolling):http://design-system-react-components.herokuapp.com/?path=/story/sldsdatatable--advanced-fixed-layout but you don't have to do this:http://design-system-react-components.herokuapp.com/?path=/story/sldsdatatable--fixed-header |
I will try to get you a new props proposal by tomorrow, but feel free to ask questions tonight and maybe we can discuss tomorrow in the meeting. |
So far this is what I have: |
aswinshenoy commentedSep 12, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
In the single select variant, I have used It would be better if could adopt the following Datatable single select design pattern for Treegrid against the current SLDS pattern. |
Yes. That's correct. Please think of the selection as flat even if the data is hierarchal--therefore only use indeterminate on the "Select All" checkbox which is how DataTable works.
Yes. That's correct. There are some differing opinions on the select all interaction pattern. I'd would like to see Select All select all the rows in the table (even the hidden ones). Since updating the data would be handled in the example code anyway, if that pattern changes in the future, it's not a big deal--we'd just update the example--but it's good to provide a good default, because everyone loves to copy and paste! 😄 One way to think about this mash up of a DataTable and Tree here at Salesforce is that it's often a database union of a table with itself. That is take the ACCOUNTS table and merge it with same ACCOUNTS table based on COLUMN A which have the same value, so the data is visibly hierarchal, but you could also, flatten the data and add a column that "links the child/parent relationships." Another way to think of it is as file tree. Selection of a folder is not visible UI selection of the children. |
As for clicking the row, in the Tree there is selection and example in separate DOM nodes. What is the difference here? We need to keep single selection and simply a treegrid and not the merger of a DataTable and a TreeGrid that is the multiple selection variant. |
I am still unable to fix the issue of
|
aswinshenoy commentedSep 13, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I have used event trap for selection and expansion handlers, and everything seems to be working fine now. In case of the checkbox, I had made the example exactly as you had said, so it didn't require any changes. I am yet to touch the switching part between Navigation Mode <-> Selection Mode. As far as I learnt: -
However, I have a slight confusion, regarding what objects will be focused I think we would be going with hovering each and every column in row (maybe except columns with some PS: I will start with browser testing later today, as rendering and functionality is almost done :) |
This issue has been automatically marked as stale, because it has not had recent activity. It will be closed if no further activity occurs. Maintainers are responsible for tech debt and project health. This is most likely a new components or component feature request. Please submit a pull request for or request feedback on this feature. Thank you. |
This issue has been automatically marked as stale, because it has not had recent activity. It will be closed if no further activity occurs. Maintainers are responsible for tech debt and project health. This is most likely a new components or component feature request. Please submit a pull request for or request feedback on this feature. Thank you. |
This issue has been automatically marked as stale, because it has not had recent activity. It will be closed if no further activity occurs. Maintainers are responsible for tech debt and project health. This is most likely a new components or component feature request. Please submit a pull request for or request feedback on this feature. Thank you. |
[8]ページ先頭
Fixes#1836
CONTRIBUTOR checklist (do not remove)
Please complete for every pull request
npm run lint:fixhas been run and linting passes.components/component-docs.jsonCI tests pass (npm test).REVIEWER checklist (do not remove)
components/component-docs.jsontests.Required only if there are markup / UX changes
last-slds-markup-reviewinpackage.jsonand push.last-accessibility-review, topackage.jsonand push.npm run local-updatewithin locally clonedsite repo to confirm the site will function correctly at the next release.