Skip to content
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

Data Table Fixes #1104

Merged
merged 3 commits into from
Dec 8, 2024
Merged

Data Table Fixes #1104

merged 3 commits into from
Dec 8, 2024

Conversation

paustint
Copy link
Contributor

@paustint paustint commented Dec 7, 2024

shift+click on a row will select multiple rows

Navigating with keyboard on checkbox column, space can now be used to select the checkbox and focus does not go crazy

Triggering edit with the keyboard now works properly in all cases regardless of the field type

Focus is now trapped in the edit popover and focus is returned when the modal closes

The right-click context menu is now properly keyboard navigable

resolves #1103

shift+click on a row will select multiple rows

Navigating with keyboard on checkbox column, space can now be used to select the checkbox and focus does not go crazy

Triggering edit with the keyboard now works properly in all cases regardless of the field type

Focus is now trapped in the edit popover and focus is returned when the modal closes

The right-click context menu is now properly keyboard navigable

resolves #1103
@paustint paustint requested a review from Copilot December 7, 2024 23:30
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 20 changed files in this pull request and generated 2 suggestions.

Files not reviewed (14)
  • .eslintrc.json: Language not supported
  • apps/jetstream/src/main.scss: Language not supported
  • package.json: Language not supported
  • libs/ui/src/lib/popover/ContextMenu.tsx: Evaluated as low risk
  • libs/ui/src/lib/data-table/SalesforceRecordDataTable.tsx: Evaluated as low risk
  • libs/ui/src/lib/data-table/DataTableSubqueryRenderer.tsx: Evaluated as low risk
  • libs/features/platform-event-monitor/src/PlatformEventMonitorEvents.tsx: Evaluated as low risk
  • libs/features/deploy/src/utils/deploy-metadata.utils.tsx: Evaluated as low risk
  • libs/ui/src/lib/data-table/data-table-utils.tsx: Evaluated as low risk
  • libs/ui/src/lib/widgets/Tooltip.tsx: Evaluated as low risk
  • libs/ui/src/lib/data-table/DataTableRenderers.tsx: Evaluated as low risk
  • libs/ui/src/index.ts: Evaluated as low risk
  • libs/ui/src/lib/form/date/DateGrid.tsx: Evaluated as low risk
  • libs/ui/src/lib/data-table/DataTableEditors.tsx: Evaluated as low risk
Comments skipped due to low confidence (2)

libs/types/src/lib/ui/types.ts:388

  • [nitpick] Consider using an enum for the 'type' property to ensure it only accepts specific values.
type: string;

libs/types/src/lib/ui/types.ts:389

  • [nitpick] Consider using an enum for the 'icon' property to ensure it only accepts specific values.
icon: string;

}, [focusedItem, items, selectedItem]);

function handleKeyDown(event: KeyboardEvent<HTMLAnchorElement>) {
event.preventDefault();
Copy link
Preview

Copilot AI Dec 7, 2024

Choose a reason for hiding this comment

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

Calling event.preventDefault() and event.stopPropagation() at the beginning of handleKeyDown might prevent default behavior for keys that are not handled by the function, such as the Tab key. Move these calls inside the conditional blocks where specific keys are handled.

Suggested change
event.preventDefault();
if (isEscapeKey(event)) { event.preventDefault(); event.stopPropagation(); onClose(); return; }

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
libs/ui/src/lib/data-table/useDataTable.tsx Show resolved Hide resolved
Copy link

Report too large to display inline

View full report↗︎

Copy link

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSourceCI
Possible typosquat attack npm/[email protected] ⚠︎
Critical CVE npm/[email protected] ⚠︎
Critical CVE npm/[email protected] ⚠︎
Critical CVE npm/[email protected] ⚠︎
Critical CVE npm/[email protected] ⚠︎
Critical CVE npm/[email protected] ⚠︎
Critical CVE npm/[email protected] ⚠︎

View full report↗︎

Next steps

What is a typosquat?

Package name is similar to other popular packages and may not be the package you want.

Use care when consuming similarly named packages and ensure that you did not intend to consume a different package. Malicious packages often publish using similar names as existing popular packages.

What is a critical CVE?

Contains a Critical Common Vulnerability and Exposure (CVE).

Remove or replace dependencies that include known critical CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@paustint paustint merged commit d75f84b into main Dec 8, 2024
6 checks passed
@paustint paustint deleted the bug/1103-data-table-fixes branch December 8, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data table keyboard navigation issues
1 participant