-
Notifications
You must be signed in to change notification settings - Fork 3
Create column menu #174
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
Open
rembrandtreyes
wants to merge
44
commits into
hyparam:master
Choose a base branch
from
rembrandtreyes:rembrandtreyes/ColumnMenuOnly
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Create column menu #174
Changes from all commits
Commits
Show all changes
44 commits
Select commit
Hold shift + click to select a range
be48281
Add column menu functionality to table headers
rembrandtreyes cfe0102
Enhance ColumnMenu with sorting functionality and refactor rendering …
rembrandtreyes 51c5a41
Refactor ColumnMenu and integrate PortalContainer for improved rendering
rembrandtreyes 2e1dcd7
Enhance HighTable CSS for improved sorting visuals
rembrandtreyes ecfb269
Refactor ColumnHeader and HighTable styles for improved usability
rembrandtreyes b17d387
Enhance ColumnMenu and ColumnMenuButton for improved accessibility an…
rembrandtreyes 2231d6c
Improve ColumnMenuButton interaction and positioning
rembrandtreyes a907f22
Enhance ColumnHeader and ColumnMenu integration for improved function…
rembrandtreyes cfcdbef
Enhance ColumnMenu and ColumnMenuButton for improved focus management
rembrandtreyes d73acee
Refactor ColumnHeader and ColumnMenu for improved event handling and …
rembrandtreyes 9059a97
Add tests for ColumnMenu and ColumnMenuButton components
rembrandtreyes 8da548a
Update sortable prop handling in ColumnHeader for consistency
rembrandtreyes f397c61
Merge remote-tracking branch 'upstream/master' into rembrandtreyes/Co…
rembrandtreyes 45d40fd
cleanup styles
rembrandtreyes 9f77d7b
Add default props for ColumnHeader tests
rembrandtreyes 56f76ac
Apply suggestions from code review
rembrandtreyes 7c3c9be
Refactor HighTable to utilize usePortalContainer hook
rembrandtreyes 6054ef0
Refactor ColumnHeader and ColumnMenu for improved state management an…
rembrandtreyes 66da8cd
Enhance ColumnMenu functionality and styles
rembrandtreyes f27d220
Implement MenuItem and Overlay components in ColumnMenu
rembrandtreyes ff7f80b
Enhance ColumnMenu focus management and keyboard navigation
rembrandtreyes 43b7d39
Enhance ColumnHeader toggle functionality
rembrandtreyes 8906fd0
Enhance ColumnMenuButton accessibility and ColumnHeader functionality
rembrandtreyes eb3332b
Merge remote-tracking branch 'upstream/master' into rembrandtreyes/Co…
rembrandtreyes c11e9cc
Remove unused `buttonRef` prop from ColumnHeader and ColumnMenu compo…
rembrandtreyes b1c9dd9
Refactor ColumnHeader and ColumnMenu for improved functionality and a…
rembrandtreyes 82498bd
Update HighTable styles for improved focus visibility
rembrandtreyes e01a906
Enhance accessibility and styles in ColumnHeader and HighTable
rembrandtreyes cec8566
Refactor ColumnMenu and TableHeader tests for improved clarity and fu…
rembrandtreyes 3d5dc25
Update HighTable styles for sort arrow positioning
rembrandtreyes 795da76
Refactor imports in HighTable and TableHeader components for cleaner …
rembrandtreyes b11258c
Refactor ColumnMenu to utilize useScrollLock hook for scroll management
rembrandtreyes 62887fc
Implement focus management in ColumnMenu using useFocusManagement hook
rembrandtreyes 3191a6e
Enhance ColumnHeader and ColumnMenuButton for improved accessibility …
rembrandtreyes 5ec8d2f
Refactor ColumnHeader to utilize useColumnMenu hook for improved func…
rembrandtreyes 1a65e7b
Update tests for ColumnHeader, ColumnMenuButton, and TableHeader for …
rembrandtreyes e2ad3ab
Refactor TableHeader tests to enhance column menu integration and acc…
rembrandtreyes 701dc1f
Add onEscape handler to ColumnMenuButton for improved keyboard naviga…
rembrandtreyes fffbd0e
Enhance ColumnMenu tests for improved coverage and functionality
rembrandtreyes e6269a8
Refactor ColumnMenu tests for consistency and readability
rembrandtreyes 26711c8
Refactor ColumnMenuButton to use button element for improved accessib…
rembrandtreyes 57d6bc8
Merge remote-tracking branch 'upstream/master' into rembrandtreyes/Co…
rembrandtreyes 59c1ee8
Update ColumnHeader and ColumnMenuButton to use button elements for i…
rembrandtreyes ea8981a
Update ColumnHeader and ColumnMenuButton tests for improved accuracy
rembrandtreyes File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,11 @@ import { flushSync } from 'react-dom' | |
import { Direction } from '../../helpers/sort.js' | ||
import { getOffsetWidth } from '../../helpers/width.js' | ||
import { useCellNavigation } from '../../hooks/useCellsNavigation.js' | ||
import ColumnMenu from '../ColumnMenu/ColumnMenu.js' | ||
import ColumnMenuButton from '../ColumnMenuButton/ColumnMenuButton.js' | ||
import { useColumnStates } from '../../hooks/useColumnStates.js' | ||
import ColumnResizer from '../ColumnResizer/ColumnResizer.js' | ||
import { useColumnMenu } from '../../hooks/useColumnMenu.js' | ||
|
||
interface Props { | ||
columnIndex: number // index of the column in the dataframe (0-based) | ||
|
@@ -22,7 +25,10 @@ interface Props { | |
|
||
export default function ColumnHeader({ columnIndex, columnName, dataReady, direction, onClick, orderByIndex, orderBySize, ariaColIndex, ariaRowIndex, className, children }: Props) { | ||
const ref = useRef<HTMLTableCellElement>(null) | ||
const buttonRef = useRef<HTMLButtonElement>(null) | ||
const { tabIndex, navigateToCell } = useCellNavigation({ ref, ariaColIndex, ariaRowIndex }) | ||
const { isOpen, position, menuId, handleToggle, handleMenuClick } = | ||
useColumnMenu(columnIndex, ref, navigateToCell) | ||
Comment on lines
+30
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add useColumnMenu hook to separate concerns ColumnHeader was handling both column display and menu logic. This hook isolates menu state management, making the component easier to understand and test. |
||
const handleClick = useCallback(() => { | ||
navigateToCell() | ||
onClick?.() | ||
|
@@ -111,6 +117,7 @@ export default function ColumnHeader({ columnIndex, columnName, dataReady, direc | |
aria-description={description} | ||
aria-rowindex={ariaRowIndex} | ||
aria-colindex={ariaColIndex} | ||
aria-expanded={isOpen} | ||
tabIndex={tabIndex} | ||
title={description} | ||
onClick={handleClick} | ||
|
@@ -120,13 +127,34 @@ export default function ColumnHeader({ columnIndex, columnName, dataReady, direc | |
data-fixed-width={dataFixedWidth} | ||
> | ||
{children} | ||
{sortable && | ||
<ColumnMenuButton | ||
ref={buttonRef} | ||
onClick={handleMenuClick} | ||
onEscape={navigateToCell} | ||
tabIndex={tabIndex} | ||
isExpanded={isOpen} | ||
menuId={menuId} | ||
aria-label={`Column menu for ${columnName}`} | ||
/> | ||
} | ||
<ColumnResizer | ||
forceWidth={forceColumnWidth} | ||
autoResize={autoResize} | ||
width={width} | ||
tabIndex={tabIndex} | ||
navigateToCell={navigateToCell} | ||
/> | ||
<ColumnMenu | ||
columnName={columnName} | ||
isOpen={isOpen} | ||
position={position} | ||
direction={direction} | ||
sortable={sortable} | ||
onClick={onClick} | ||
columnIndex={columnIndex} | ||
onToggle={handleToggle} | ||
/> | ||
</th> | ||
) | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Needed to bump this to 24px to now account for the sorting arrows and column menu button