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

fix: navigate with arrow keys in select/multi-select #5983

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AdityaPimpalkar
Copy link
Contributor

closes: #4977

Screen.Recording.2024-06-21.at.3.24.46.PM.mov

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

  • Added arrow key navigation support in ObjectFilterDropdownRecordSelect.tsx
  • Integrated SelectableList component in MultipleRecordSelectDropdown.tsx
  • Managed selected item state using Recoil in MultipleRecordSelectDropdown.tsx
  • Applied background color for isKeySelected prop in StyledMenuItemBase.tsx

3 file(s) reviewed, no comment(s)

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

  • Introduced RelationFilter type for enhanced GraphQL filtering
  • Added isEmptyOperand support in MultipleFiltersDropdownContent.tsx
  • Renamed and improved handleOperandChange in ObjectFilterDropdownOperandSelect.tsx
  • Enhanced test coverage in getOperandsForFilterType.test.tsx
  • Added onSubmit prop to DateFieldInput and DateTimeFieldInput components

36 file(s) reviewed, 1 comment(s)

Comment on lines 38 to 39
${({ isHoverBackgroundDisabled }) =>
isHoverBackgroundDisabled ?? HOVER_BACKGROUND};
Copy link

Choose a reason for hiding this comment

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

Logical error: isHoverBackgroundDisabled should be checked for truthiness before applying HOVER_BACKGROUND.

Suggested change
${({ isHoverBackgroundDisabled }) =>
isHoverBackgroundDisabled ?? HOVER_BACKGROUND};
${({ isHoverBackgroundDisabled }) =>
!isHoverBackgroundDisabled && HOVER_BACKGROUND};

@lucasbordeau
Copy link
Contributor

  • Enter should select the hovered item, here it opens a drawer.
  • We should fix it for multi select also in task relations for example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

I should be able to navigate with arrow keys in select/multi-select
3 participants