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

Feature/355 select multiple component #385

Merged
merged 18 commits into from
May 6, 2024

Conversation

g-saracca
Copy link
Contributor

@g-saracca g-saracca commented Apr 29, 2024

What this PR does / why we need it:
Adds new Select Multiple component to the Design System.

Which issue(s) this PR closes:
Closes #355

Special notes for your reviewer:
Also export some form elements interfaces to solve a ts errors issue when building the design system about FormGroup trying to access private types.
Also add props to existent Select component.

Suggestions on how to test this:

Test the component in storybook.
Check Isolated Select Multiple component in Storybook

Check Select Multiple component within a form in Storybook.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Screenshot 2024-04-29 at 14 43 10

Is there a release notes update needed for this change?:
CHANGELOG in the Design System package has been updated.

Additional documentation:
No.

@coveralls
Copy link

coveralls commented Apr 29, 2024

Coverage Status

coverage: 97.424% (+0.09%) from 97.335%
when pulling 5a12f28 on feature/355-select-multiple-component
into 774661b on develop.

@g-saracca g-saracca added Size: 3 A percentage of a sprint. 2.1 hours. D: Design System Deliverable: Design System pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows pm.GREI-d-2.7.2 NIH, yr2, aim7, task2: Implement UI modules for creating datasets and publishing workflows labels Apr 29, 2024
@g-saracca g-saracca marked this pull request as ready for review April 29, 2024 17:55
@ekraffmiller ekraffmiller self-assigned this May 2, 2024
Copy link
Contributor

@ekraffmiller ekraffmiller left a comment

Choose a reason for hiding this comment

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

This looks really good! I don't have any requested changes in the code, but I noticed a behavior in the Story that I think could be improved. When you type in the search to get a subset of items, then click the select all checkbox, it selects all the items in the list, not just what you selected, which seems a little unexpected.
For example:

Screen.Recording.2024-05-02.at.9.10.55.PM.mov

@g-saracca
Copy link
Contributor Author

Nice catch ellen! thanks!
I've refactored the way it behaves a bit, covering more edge cases and added more tests for each of them.

@g-saracca g-saracca removed their assignment May 3, 2024
Copy link
Contributor

@ekraffmiller ekraffmiller left a comment

Choose a reason for hiding this comment

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

looks good! approved!

@ekraffmiller ekraffmiller removed their assignment May 3, 2024
@GPortas GPortas self-assigned this May 6, 2024
Copy link
Contributor

@GPortas GPortas left a comment

Choose a reason for hiding this comment

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

I'm facing the same error that @ekraffmiller initially detected. When I select all items based on a query string, all items without filter are selected. This happens in both stories.

Screen.Recording.2024-05-06.at.10.48.13.mov
Screen.Recording.2024-05-06.at.10.50.54.mov

@g-saracca
Copy link
Contributor Author

Hi @GPortas , the initial storybooks links from the PR description are outdated, they have the code before the changes requested by @ekraffmiller.
Here is the last storybook build with the latest changes, please review it here.
I was really convinced that action of chromatic deployment running on same PR was going to replace latest build on the same url than before.

@g-saracca g-saracca removed their assignment May 6, 2024
@GPortas GPortas merged commit 4fa9b65 into develop May 6, 2024
13 of 14 checks passed
@GPortas GPortas deleted the feature/355-select-multiple-component branch May 6, 2024 12:47
jayanthkomarraju pushed a commit to jayanthkomarraju/dataverse-frontend that referenced this pull request May 31, 2024
…ponent

Feature/355 select multiple component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D: Design System Deliverable: Design System pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows pm.GREI-d-2.7.2 NIH, yr2, aim7, task2: Implement UI modules for creating datasets and publishing workflows Size: 3 A percentage of a sprint. 2.1 hours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Select multiple component for the design system
4 participants