Skip to content
This repository has been archived by the owner on Nov 13, 2024. It is now read-only.

DEX-922 implement stickyHeader and add a meaningful maxHeight to ever… #430

Merged
merged 7 commits into from
Jul 25, 2022
Merged

DEX-922 implement stickyHeader and add a meaningful maxHeight to ever… #430

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a43d331
DEX-922 implement stickyHeader and add a meaningful maxHeight to ever…
Atticus29 Jul 13, 2022
a12ea15
DEX-922 remove seemingly unnecessary horizontal scroll prop
Atticus29 Jul 13, 2022
1873372
DEX-922 respond to code review feedback. Remove default maxHeight for…
Atticus29 Jul 21, 2022
c1cb541
DEX-922 respond to code review feedback pt 2
Atticus29 Jul 22, 2022
47f19e8
DEX-922 add tableContainerStyles to DataDisplay and remove maxHeight
Atticus29 Jul 25, 2022
d9f0f7b
DEX-922 remove tableContainerStyles from SightingsDisplay and fix a f…
Atticus29 Jul 25, 2022
ffcc2ad
DEX-922 final little fixes
Atticus29 Jul 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/IndividualSelector.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export default function IndividualSelector({
{canShowTable && (
<DataDisplay
idKey="guid"
paperStyles={{ maxHeight: 360 }}
maxHeight={360}
style={{ marginTop: 12 }}
noTitleBar
variant="secondary"
Expand Down
1 change: 1 addition & 0 deletions src/components/cards/CollaborationsCard.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export default function CollaborationsCard({
columns={columns}
data={tableData}
idKey="guid"
maxHeight={600}
/>
</Card>
);
Expand Down
1 change: 1 addition & 0 deletions src/components/cards/CooccurrenceCard.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export default function CooccurrenceCard({
},
]}
data={data}
maxHeight={600}
/>
</Card>
);
Expand Down
1 change: 1 addition & 0 deletions src/components/cards/EncountersCard.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ export default function EncountersCard({
columns={filteredColumns}
data={encountersWithLocationData}
tableStyles={{ tableLayout: 'fixed' }}
maxHeight={600}
/>
)}
{!noEncounters && showMapView && <div>Map goes here</div>}
Expand Down
1 change: 1 addition & 0 deletions src/components/cards/SightingsCard.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export default function SightingsCard({
data={sightings}
loading={loading}
noResultsTextId={noSightingsMsg}
maxHeight={400}
/>
)}
{!noSightings && showMapView && (
Expand Down
8 changes: 7 additions & 1 deletion src/components/dataDisplays/DataDisplay.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ export default function DataDisplay({
paperStyles = {},
Emily-Ke marked this conversation as resolved.
Show resolved Hide resolved
tableStyles = {},
cellStyles = {},
stickyHeader = true,
maxHeight,
...rest
}) {
const intl = useIntl();
Expand Down Expand Up @@ -258,9 +260,13 @@ export default function DataDisplay({
<TableContainer
component={variant === 'secondary' ? Paper : undefined}
elevation={variant === 'secondary' ? 2 : undefined}
style={paperStyles}
style={{
maxHeight,
...paperStyles,
}}
>
<Table
stickyHeader={stickyHeader}
style={{ minWidth: 10, ...tableStyles }}
size={tableSize}
aria-label={title}
Expand Down
1 change: 1 addition & 0 deletions src/components/dataDisplays/IndividualsDisplay.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export default function IndividualsDisplay({
data={individuals}
title={title}
loading={loading}
maxHeight={600}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be a maxHeight on paginated tables. If we want the table to be shorter, shouldn't we just reduce the request page size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. Are we convinced that every instantiation of IndividualsDisplay is going to be paginated?

Copy link
Contributor

Choose a reason for hiding this comment

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

IndividualsDisplay passes its rest props to its DataDisplay component, so if an instance would rather have a maximum height than be paginated, it can pass maxHeight.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I've noticed two weird things now:

  1. When passing maxHeight to IndividualsDisplay, the DataDisplay does NOT seem to be respecting it:
    <IndividualsDisplay
    individuals={searchResults || []}
    hideFilterSearch
    loading={loading}
    sortExternally
    searchParams={searchParams}
    setSearchParams={setSearchParams}
    dataCount={resultCount}
    style={{ maxHeight: 600 }}
    />

  2. The limit: 20 param in useFilterIndividuals.js also seems not to be respected. I created an investigation ticket for this latter one: DEX-1322.

Copy link
Contributor Author

@Atticus29 Atticus29 Jul 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Removed in c1cb541. So, currently that means that this table is displaying a ton of rows.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the first weird thing: You are not technically passing maxHeight to IndividualsDisplay, you are passing style. Neither of those two props are used directly by IndividualsDisplay. Instead they are destructured into its rest property and spread as props to DataDisplay. DataDisplay does destructure maxHeight, but style is once again destructured into the rest property. The maxHeight prop is used in the TableContainer's style, but rest is spread to the div wrapping all of DataDisplay's other children. That div has the default overflow: visible, so it may not look like maxHeight is working, but it is.

When I pass style={{ maxHeight: 200, border: '1px solid red' }} to IndividualsDisplay, it is only 200px tall, but it overflows. Notice that the pagination is right below the 200px tall display:
max-height-wrapper

When I pass maxHeight={200} to IndividualsDisplay, the table height is constrained:
max-height-table

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe instead of adding a maxHeight prop to DataDisplay, we should add a tableContainerStyles prop instead?

// onPrint={() => {
// window.open('/individuals/picturebook', '_blank');
// }}
Expand Down
1 change: 1 addition & 0 deletions src/components/dataDisplays/RelationshipDisplay.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export default function RelationshipDisplay({
columns={relationshipCols}
data={data}
loading={loading}
maxHeight={400}
Emily-Ke marked this conversation as resolved.
Show resolved Hide resolved
/>
);
}
1 change: 1 addition & 0 deletions src/components/dataDisplays/SightingsDisplay.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export default function SightingsDisplay({
title={title}
loading={loading}
showNoResultsBao
maxHeight={600}
// renderExpandedRow={expandedSighting => (
// <div style={{ display: 'flex' }}>
// <img
Expand Down
1 change: 1 addition & 0 deletions src/pages/assetGroup/AGSTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export default function AGSTable({ assetGroupSightings }) {
data={transformedData}
columns={columns}
style={{ marginTop: 20 }}
maxHeight={600}
/>
);
}
1 change: 1 addition & 0 deletions src/pages/fieldManagement/settings/CategoryTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ export default function FieldSettings() {
variant="secondary"
columns={categoryColumns}
data={categories}
maxHeight={300}
/>
</Grid>
);
Expand Down
1 change: 1 addition & 0 deletions src/pages/fieldManagement/settings/CustomFieldTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ export default function CustomFieldTable({
columns={tableColumns}
data={fieldSchemas}
noResultsTextId={noFieldsTextId}
maxHeight={300}
/>
</Grid>
);
Expand Down
1 change: 1 addition & 0 deletions src/pages/fieldManagement/settings/DefaultFieldTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ export default function DefaultFieldTable({
variant="secondary"
columns={tableColumns}
data={configurableFields}
maxHeight={300}
/>
</Grid>
);
Expand Down
1 change: 1 addition & 0 deletions src/pages/fieldManagement/settings/defaultFieldComponents/SpeciesEditor.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ export default function SpeciesEditor({
columns={tableColumns}
data={searchResults || suggestedValues}
idKey="itisTsn"
maxHeight={300}
Emily-Ke marked this conversation as resolved.
Show resolved Hide resolved
/>
<Text
component="p"
Expand Down
1 change: 1 addition & 0 deletions src/pages/match/MatchCandidatesTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export default function MatchCandidatesTable({
setSelectedMatchCandidate(nextSelection);
}
}}
maxHeight={300}
/>
);
}
1 change: 1 addition & 0 deletions src/pages/match/QueryAnnotationsTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export default function QueryAnnotationsTable({
setSelectedQueryAnnotation(nextSelection);
}
}}
maxHeight={300}
/>
);
}
1 change: 1 addition & 0 deletions src/pages/userManagement/UserEditTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export default function UserEditTable() {
columns={tableColumns}
data={activeUsers}
loading={loading}
maxHeight={500}
/>
{usersError ? (
<Text
Expand Down
1 change: 1 addition & 0 deletions src/pages/userManagement/UserManagerCollaborationEditTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ export default function UserManagersCollaborationEditTable({
columns={tableColumns}
data={tableFriendlyData || []}
noResultsTextId="NO_COLLABORATIONS_MESSAGE"
maxHeight={500}
/>
{collaborationError ? (
<Text
Expand Down