Skip to content

Sticky header using intersection observer for recordset page #2608

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.

Sign up for GitHub

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

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Sticky header using intersection observer for recordset page #2608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e4366eb
sticky-header-intersection-observer
KenyShah24 Dec 11, 2024
35cfd8b
Cleanup code
KenyShah24 Jan 22, 2025
1061ab1
Merge branch 'master' of https://github.com/informatics-isi-edu/chais…
KenyShah24 Jan 27, 2025
314c1d9
Fix test for recorset table header
KenyShah24 Feb 4, 2025
a6b4083
Merge branch 'master' of https://github.com/informatics-isi-edu/chais…
KenyShah24 Feb 4, 2025
084ab9d
Reverse playwright setup changes
KenyShah24 Feb 4, 2025
b5f6adc
Fix 2 locators for recordset
KenyShah24 Feb 10, 2025
2fb2a86
Fix locators for recordset
KenyShah24 Feb 10, 2025
e7a17f3
Add new node version for Docker that's compatible with project
KenyShah24 Feb 11, 2025
f6e1cf6
CLeanup
KenyShah24 Feb 11, 2025
d51e1f5
Different approach for calculating header top to show sticky header f…
KenyShah24 Mar 26, 2025
ae4f216
Approach refined as per comments
KenyShah24 Apr 1, 2025
4459ba9
Removed redundant styles
KenyShah24 Apr 1, 2025
60bb669
Remove redundant styles
KenyShah24 Apr 1, 2025
60a366d
Cleanup files
KenyShah24 Apr 1, 2025
08c8c06
Cleanup files
KenyShah24 Apr 1, 2025
ca340f9
Add resize observer to sync width
KenyShah24 Apr 15, 2025
a5fba7c
Added comments wherever needed and variable name changes
KenyShah24 Apr 16, 2025
f2dfff2
Added comments wherever needed and variable name changes
KenyShah24 Apr 16, 2025
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
25 changes: 23 additions & 2 deletions src/assets/scss/_recordset-table.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
overflow-y: hidden;
}

.chaise-table.table {
.chaise-table.table, .sticky-header > table {
border-top: 1px solid map.get(variables.$color-map, 'table-border');
border-bottom: 1px solid map.get(variables.$color-map, 'table-border');
margin: 0;
Expand Down Expand Up @@ -259,6 +259,27 @@
top: 0;
}
.no-scroll-bar {
display: none !important;
display: none !important;
position: absolute !important;
}
.sticky-header {
Copy link
Member

Choose a reason for hiding this comment

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

Please rename this to chaise-table-sticky-header to be more specific.

position: fixed;
top: 0;
visibility: none;
box-shadow: 0 2px 5px rgba(0, 0, 0, 0.1);
overflow-x: hidden;
}

.sticky-header > table > thead > tr > th {
padding: 7px;
color: map.get(variables.$color-map, 'navbar-inverse');
}

.sticky-header .sticky-header-table {
table-layout: fixed;
width: 100%;
}

.app-content-container-scrollable .sticky-header {
display: none;
}
105 changes: 95 additions & 10 deletions src/components/recordset/recordset-table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@ type RecordsetTableProps = {
* Determines if both horizontal scrollbars should always be visible, or if only one should appear at a time.
*/
showSingleScrollbar?: boolean,
// Determines the top for the sticky header
Copy link
Member

Choose a reason for hiding this comment

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

Use JSDoc style of comment like the previous prop that you see. So

/**
 * Determines the top for the sticky header
 */
headerTop?: number,

headerTop?: number,
sortCallback?: (sortColumn: SortColumn) => any
}

const RecordsetTable = ({
config,
initialSortObject,
showSingleScrollbar = false
showSingleScrollbar = false,
headerTop,
}: RecordsetTableProps): JSX.Element => {

const {
Expand All @@ -60,6 +63,10 @@ const RecordsetTable = ({
const tableContainer = useRef<HTMLDivElement>(null);
const stickyScrollbarRef = useRef<HTMLDivElement>(null);
const tableEndRef = useRef<HTMLDivElement>(null);
const tableRef = useRef<HTMLTableElement>(null);
const outerTableRef = useRef<HTMLDivElement>(null);
const headRef = useRef<HTMLTableSectionElement>(null);
const stickyHeaderRef = useRef<HTMLDivElement>(null);


const [currSortColumn, setCurrSortColumn] = useState<SortColumn | null>(
Expand Down Expand Up @@ -157,10 +164,75 @@ const RecordsetTable = ({
}

return () => {
observer.disconnect();
observer.disconnect();
}
}, []);

//To handle sticky header visibility based on element intersection
useLayoutEffect(() => {
if (!outerTableRef.current || !headRef.current || !stickyHeaderRef.current || !stickyScrollbarRef.current) {
return;
}

// Create an IntersectionObserver to track when the table header is visible
const observer = new IntersectionObserver(
([entry]) => {
if (stickyHeaderRef.current) {
if (!entry.isIntersecting) {
stickyHeaderRef.current.style.visibility = 'visible';
// Adjust the sticky header position based on the presence of a scrollbar height and top panel container's height
const scrollbarHeight = stickyScrollbarRef.current?.offsetHeight || 0;
stickyHeaderRef.current.style.top=`${headerTop ? headerTop + scrollbarHeight:0}px`;
} else {
stickyHeaderRef.current.style.visibility = 'hidden';
}
}
},
{ root: null, threshold: 0 }
);
observer.observe(headRef.current);

// Sync widths of the columns
const syncWidths = () => {
if (stickyHeaderRef.current && tableRef.current) {

const originalThs = tableRef.current.querySelectorAll('tbody > tr > td');
Copy link
Member

Choose a reason for hiding this comment

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

Why is this selecting tds? and not th?

const stickyThs = stickyHeaderRef.current?.querySelectorAll('th');

// Loop through columns and set widths
stickyThs!.forEach((headerCol, index) => {
const dataCol = originalThs[index];
if (dataCol instanceof HTMLElement) {
const colWidth = dataCol.offsetWidth; // Get the actual width of the column
headerCol.style.width = `${colWidth}px`; // Set width on sticky header
}

});
stickyHeaderRef.current.style.width = `${outerTableRef.current?.offsetWidth}px`;
}
};

// Function to synchronize the horizontal scroll position of the sticky header with the scrollbar
const handleScroll = () => {
if (stickyHeaderRef.current && stickyScrollbarRef.current) {
stickyHeaderRef.current.scrollLeft = stickyScrollbarRef.current.scrollLeft;
}
};

stickyScrollbarRef.current?.addEventListener('scroll', handleScroll);

// Sync column widths on resize
const headerWidthResizeObserver = new ResizeObserver(syncWidths);
headerWidthResizeObserver.observe(outerTableRef.current);

// Cleanup function
return () => {
observer.disconnect();
stickyScrollbarRef.current?.removeEventListener('scroll', handleScroll);
headerWidthResizeObserver.disconnect();
};

}, [isInitialized, headerTop]);

/**
* add the top horizontal scroll if needed
Expand Down Expand Up @@ -423,6 +495,15 @@ const RecordsetTable = ({
})
}

const renderHeader = () => {
return(
<tr>
Copy link
Member

Choose a reason for hiding this comment

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

Fix the indentation to match the rest of the file.

{showActionButtons && renderActionsHeader()}
{renderColumnHeaders()}
</tr>
);
}

const renderRows = () => {
if (hasTimeoutError) {
return (
Expand Down Expand Up @@ -519,13 +600,10 @@ const RecordsetTable = ({
className='chaise-table-top-scroll-wrapper'>
<div className='chaise-table-top-scroll'></div>
</div>
<div className={outerTableClassname()}>
<table className='table chaise-table table-hover'>
<thead className='table-heading'>
<tr>
{showActionButtons && renderActionsHeader()}
{renderColumnHeaders()}
</tr>
<div className={outerTableClassname()} ref={outerTableRef}>
<table className='table chaise-table table-hover' ref={tableRef}>
<thead className='table-heading' ref={headRef}>
{renderHeader()}
</thead>
<tbody>
{renderRows()}
Expand All @@ -534,7 +612,14 @@ const RecordsetTable = ({
</div>
{/* This div will be used as the target (end of table) for the intersection observer to hide the
top scrollbar when the bottom one is visible */}
<div className='dummy-table-end-div' ref={tableEndRef}/>
<div className='dummy-table-end-div' ref={tableEndRef} />
{config.displayMode.indexOf(RecordsetDisplayMode.RELATED) !== 0 && <div className='sticky-header' id='sticky-header' ref={stickyHeaderRef}>
Copy link
Member

Choose a reason for hiding this comment

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

Create a variable for config.displayMode.indexOf(RecordsetDisplayMode.RELATED) !== 0 at the beginning of this component. Call it something like enableStickyHeader. And then use this boolean for all the different places that you have the sticky header logic.

Copy link
Member

Choose a reason for hiding this comment

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

Remove the id attribute.

<table className='sticky-header-table'>
<thead className='table-heading sticky'>
{renderHeader()}
</thead>
</table>
</div>}

{!hasTimeoutError && numHiddenRecords > 0 &&
<div className='chaise-table-footer'>
Expand Down
6 changes: 5 additions & 1 deletion src/components/recordset/recordset.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ const RecordsetInner = ({

const [savedQueryUpdated, setSavedQueryUpdated] = useState<boolean>(false);

const [topPanelDimensions, setTopPanelDimensions] = useState<any>({});

const [permalinkTooltip, setPermalinkTooltip] = useState(MESSAGE_MAP.tooltip.permalink);

const mainContainer = useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -367,7 +369,8 @@ const RecordsetInner = ({
}

// handle the scrollable container
const resizeSensors = attachContainerHeightSensors(parentContainer, parentStickyArea);
const resizeSensors = attachContainerHeightSensors(parentContainer, parentStickyArea, undefined, setTopPanelDimensions);
Copy link
Member

Choose a reason for hiding this comment

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

The more I think about it, the more I don't like this solution. Since the callback is called on each height or width change, React is rerendering recordset component and all its children on each height/width change. So this solution would never work on record page, and it can slow the page down on long recordset pages.

As I mentioned before, we're just manipulating the styles attached to the elements, so there's no reason to involve React state in this. Instead of calling setTopPanelDimensions and passing the state variable to RecordsetTable, is there anyway that we can pass the top value from here? Maybe attach it as a data-* attribute to an element and then grab it from that page?



// log the right click event on the permalink button
const permalink = document.getElementById('permalink');
Expand Down Expand Up @@ -811,6 +814,7 @@ const RecordsetInner = ({
<RecordsetTable
config={config}
initialSortObject={initialReference.location.sortObject}
headerTop={topPanelDimensions?.top}
/>
</div>
{config.displayMode === RecordsetDisplayMode.FULLSCREEN && <Footer />}
Expand Down
12 changes: 11 additions & 1 deletion src/utils/ui-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Tooltip from 'bootstrap/js/dist/tooltip';
* @param {Node=} parentContainer - the parent container. if undefined `body` will be used.
* @param {Node=} parentContainerSticky - the sticky area of parent. if undefined `.app-header-container` will be used.
* @param {boolean} useDocHeight - whether we should use the doc height even if parentContainer is passed.
* @param {Callback Function} onContainerHeightChange - Callback allows external functions to receive height or any other dimension updates.
* Call this function once the DOM elements are loaded to attach resize sensors that will fix the height of bottom-panel-container
* If you don't pass any parentContainer, it will use the body
* It will assume the following structure in the given parentContainer:
Expand All @@ -22,7 +23,7 @@ import Tooltip from 'bootstrap/js/dist/tooltip';
*
* TODO offsetHeight is a rounded integer, should we use getBoundingClientRect().height in this function instead?
*/
export function attachContainerHeightSensors(parentContainer?: any, parentContainerSticky?: any, useDocHeight?: boolean) {
export function attachContainerHeightSensors(parentContainer?: any, parentContainerSticky?: any, useDocHeight?: boolean, onContainerHeightChange?: (dimensions: any) => void) {
Copy link
Member

Choose a reason for hiding this comment

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

Don't use any. Create a proper type here and for the state variable in src/components/recordset/recordset.tsx import that type:

export type ContainerHeightSensorDimensions = {
  /**
   * explain what it is here
   */
  top: number;
}

Also this line is now too long (you should see it being highlighted as a warning on VSCode if you have eslint installed). So please break it into multiple lines.

try {
const appRootId = `#${ID_NAMES.APP_ROOT}`;

Expand Down Expand Up @@ -53,6 +54,7 @@ export function attachContainerHeightSensors(parentContainer?: any, parentContai
appContent.style.overflowY = 'auto';
appContent.style.height = ((parentUsableHeight / windowRef.innerHeight) * 100) + 'vh';
container.style.height = 'unset';
appContent.classList.add('app-content-container-scrollable');
Copy link
Member

Choose a reason for hiding this comment

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

Add this to CLASS_NAMES in src/utils/constants.ts.

}

let tm: any;
Expand All @@ -79,12 +81,14 @@ export function attachContainerHeightSensors(parentContainer?: any, parentContai
}

const containerHeight = ((parentUsableHeight - stickyHeight) / windowRef.innerHeight) * 100;

if (containerHeight < 15) {
resetHeight();
} else {
//remove the styles that might have been added to appContent
appContent.style.overflowY = 'unset';
appContent.style.height = 'unset';
appContent.classList.remove('app-content-container-scrollable');

// set the container's height
container.style.height = containerHeight + 'vh';
Expand All @@ -94,6 +98,12 @@ export function attachContainerHeightSensors(parentContainer?: any, parentContai
resetHeight();
}
}
if (onContainerHeightChange) {
//To notify height change
onContainerHeightChange({
top: containerSticky.offsetHeight + parentContainerSticky.offsetHeight,
Copy link
Member

Choose a reason for hiding this comment

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

Instead of recalculating this, is it possible to change the code above to first calculate the top. And then sets the container height using this?

What you have is not working properly for popups. You can test this on isa:dataset table (https://dev.derivacloud.org/chaise/recordset/#1/isa:dataset@sort(accession::desc::). For example the Title facet is a good example.

});
}
}


Expand Down
Loading