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

Foreign key dropdown #2348

Merged
merged 27 commits into from
Sep 12, 2023
Merged

Foreign key dropdown #2348

merged 27 commits into from
Sep 12, 2023

Conversation

jrchudy
Copy link
Member

@jrchudy jrchudy commented Aug 22, 2023

This PR adds UI support for showing foreign key inputs in recordedit as typeahead dropdown elements. These changes go alongside changes in ermrestJS that add a new property to multiple annotations for configuring this display mode.

Changes included:

  • Added functionality for foreign key dropdown with search and load next page functionality
  • when foreign key dropdown and boolean dropdown are opened, attach a class to the recordedit-form to signal a dropdown is open. This class adds padding-bottom: 400px to extend the height of the div to allow for content to overflow down. These 2 dropdowns have a max-height of 395 px.
  • Changed a constant value that was spelled incorrectly
  • test cases in default schema for setting default properly
  • test cases for functionality of dropdown
  • include chromedriver in package.json to fix local test environments
  • added logging for each action associated with the dropdown

To test this, I added a foreign key dropdown input to the defaults schema in ErmrestDataUtils.

…to test if github actions can test properly in sauce labs with the version of chrome + chromedriver
@jrchudy jrchudy self-assigned this Aug 22, 2023
Copy link
Member

@RFSH RFSH left a comment

Choose a reason for hiding this comment

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

Functionality

After reverting the submit preventDefault change, everything works properly for me. The new test cases pass locally. That being said, we need to "invalidate" the currently fetched rows if users made any changes to the form and the current input has domain filter. The URL to fetch the rows might change if users made a change to the form and dropdownReference is still based on the initial data. To test this, we could change on of the fks in the domain-filter spec to be a dropdown.

UI

The only UI problems that I could find are related to the spinner:

  1. The foreignkeys in the "select-all-row" are not showing the spinner properly. Moving the .column-cell-spinner-container rules under .entity-value to be under the next line .entity-value,.match-entity-value will help a bit. Apart from that we should add a position: relative to the input and also adjust the top position.
  2. The spinner is not showing up during search. It's only displayed for initial load and fetching the next page.

Code

Apart from the comments that I made inline, I think we should avoid duplicating code. I understand why instead of just adding branching logic to existing ForeignkeyField you added a new ForeignkeyDropdownField, but there are a bunch of duplicated code that we should refactor.

I can think of two ways to tackle this:

  1. We could just move the three common functions to recordedit-utils.ts. Similar to how populateLinkedData is moved there.
  2. Change existing foreignkey-field.tsx to only have the common functionalities and add a new foreignkey-popup-field.tsx. Similar to how we refactored input-field.tsx.

package.json Outdated Show resolved Hide resolved
src/assets/scss/_input-switch.scss Outdated Show resolved Hide resolved
src/assets/scss/_input-switch.scss Show resolved Hide resolved
src/assets/scss/_recordedit.scss Outdated Show resolved Hide resolved
src/components/recordedit/form-container.tsx Outdated Show resolved Hide resolved
src/components/input-switch/foreignkey-dropdown-field.tsx Outdated Show resolved Hide resolved
@jrchudy jrchudy merged commit 53cdfc1 into master Sep 12, 2023
1 check passed
@jrchudy jrchudy deleted the foreign-key-dropdown branch September 12, 2023 02:21
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.

Foreign key dropdown instead of modal picker
2 participants