-
Notifications
You must be signed in to change notification settings - Fork 72
[flowsheet-data-grid] Hide columns headers #1882
Conversation
@@ -295,6 +295,7 @@ function FlowsheetDataGrid(props) { | |||
onCellSelect={handleCellSelection} | |||
onClearSelection={handleClearSelectedCells} | |||
onCellRangeSelect={handleCellRangeSelection} | |||
hasColumnHeaders={false} |
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 Flowsheet should have column headers by default. The prop should be able to be set by consumers.
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 I misunderstood the requirements then. I'll change it.
@sdadn Based on Charles's comment, and your response, looks like you are addressing this. But from a documentation/examples perspective I think we should have a specific example without column headers and then keep the others to have headers. |
* Changed | ||
* Updated FlowsheetDataGrid to no longer have column headers. |
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.
I think this will now go under the Added section. Also, you are allowing the column headers to be hidden. However, it is still possible. Maybe this update was missed based on your other updates in the PR.
import React, { useState, useCallback } from 'react'; | ||
import { FlowsheetDataGrid } from 'terra-data-grid'; | ||
|
||
const DefaultFlowsheetDataGrid = () => { |
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.
I believe this should be ColumnHeadersHidden instead of DefaultFlowsheetDataGrid.
@eawww , @chrismichalewicz I added the no header example, so it should now be ready for review. |
@sdadn Did you update the right example? Also can "Columns Header Hidden" be updated to "Column Headers Hidden"? |
@@ -376,6 +385,7 @@ const DataGrid = injectIntl((props) => { | |||
const key = event.keyCode; | |||
switch (key) { | |||
case KeyCode.KEY_UP: | |||
if (!hasVisibleColumnHeaders && nextRow - 1 === 0) break; |
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.
I would think you could move this logic to this block of code
if (nextCol < 0 || nextRow < 0) {
event.preventDefault(); // prevent the page from moving with the arrow keys.
return;
}
Your current implementation does not account for preventing the default behavior so that the page does not scroll. The basis of that logic is already lower in the file and checking for the top boundary. You would just have to modify the condition to also handle the hidden column header use case.
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.
Updated here: 766fbf5
# Hiding Column Headers | ||
|
||
### Description | ||
The [Flowsheet Data Grid](/components/cerner-terra-framework-docs/data-grid/flowsheet-data-grid/about) can hide column headers with the `hasVisibleColumnHeaders` prop. | ||
|
||
### Usage | ||
The `hasVisibleColumns` is a boolean. By default, it is set to `true`. | ||
When it is set to `false`, the column headers are hidden, but still exist in the DOM and can be read by screenreaders when providing context for the table. | ||
This is useful when needing to use custom alternative to column headers, such as Stella Timeline. | ||
|
||
**Note:** | ||
- Column header labels should still be provided in the dataset to ensure that sceenreaders will still read out the appropriate context for the table. | ||
- When using custom column headers, then logic must be added to ensure that the column widths match are in sync if resizing columns are enabled. | ||
The [columns.width](/components/cerner-terra-framework-docs/data-grid/flowsheet-data-grid/about#column) property for columns can be used to set the column widths programatically. |
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.
It seems like this should be on the About page instead of an example page. You would not want consumers to miss this information because they did not select one of the examples.
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.
For now, I'm just adding the necessary information. It can be reorganized in the implementation guide story which will be updating the documentation as a whole and making sure all pieces fit together.
…a-framework into hide-flowsheet-column-header
Summary
This PR hides the column headers in the flowsheet-data-grid to meet functional requirements.
Testing
This change was tested using:
Reviews
In addition to engineering reviews, this PR needs:
Additional Details
This PR resolves:
UXPLATFORM-9771
Thank you for contributing to Terra.
@cerner/terra