clients icon indicating copy to clipboard operation
clients copied to clipboard

[CL-27] [EC-455] Ng-Select Integration

Open vincentsalucci opened this issue 1 year ago • 0 comments

Type of change

  • [ ] Bug fix
  • [X] New feature development
  • [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • [ ] Build/deploy pipeline (DevOps)
  • [ ] Other

Objective

Integrate/wrap the ng-select library as a Component Library component for use between projects.

Code changes

  • libs/components/src/index.ts: Added ./multi-select component to barrel file
  • libs/components/src/multi-select/index.ts: Exported necessary parts of the multi-select functionality
  • libs/components/src/multi-select/models/selectItemView.ts: Created SelectItemView to be used for handling necessary data within the ng-select component. As of right now, this only supports the data types used within the Manage Organization flows.
  • libs/components/src/multi-select/multi-select.component.html: Created component layout. Passed in all necessary variables and created custom templates to match mocks.
  • libs/components/src/multi-select/multi-select.component.ts: Created corresponding component class. Handles multi-select scenarios and defaults to our implementation. Was very explicit with comments as this class works with the ng-select library and its API. We diverged a bit from standard use with how we collect the final selections.
  • libs/components/src/multi-select/multi-select.module.ts: Created module for use of the MultiSelectComponent
  • libs/components/src/multi-select/multi-select.stories.ts: Created stories depicting the usage for our current manage organization use cases: Groups Members Collections Members and Groups
  • libs/components/src/multi-select/scss/bw.theme.scss: Created customized bitwarden theme. I've left verbose comments about the selections. Other than base colors, I did not change much else and built the corresponding layouts with the default stylings.
  • libs/components/src/styles.scss: Added import for the new theme and update existing references as according to this
  • package.json: Installed the latest version of ng-select

Screenshots

0-multi-select-groups

Design Departures

  • The only real difference between the mocks and this implementation is the nested collection. I've opted to using the ng-select: groupBy API instead of trying to craft our own. Below is a screenshot of the visual implementation:

1-multi-select-groupBy

Remaining Issues/Quirks

  • @Hinton: when using ngModel to bind our returned list of selected items, we're also required to supply a name field (per angular). Right now I just have a hardcoded string to suppress the error. What's the best approach here?
  • @Hinton: I ran a quick test using the UserGroupsComponent and ran into an interesting problem: hitting enter in the dialog would end up triggering the submit action of the dialog (which would also close the dialog) while also closing the list and enacting our onItemsConfirmed emitter. Is this a setting for dialogs? Or might we need to reconsider our flow for selecting items within the ng-select component?
  • @danielleflinn: Using the groupBy API results in the collection list being "out of order" when compared to the mocks. It appears the listing will put un-grouped items first and then do the grouped items. Is this a deal breaker? Another question is in regards to the screenreader TTS when we confirm selected items: I'm unsure of how we control that and am looking for some guidance there.
  • The final note is that this component has been specifically coded to work with the multipleSelection option enabled. If we have plans for introducing/using this with single selection, we'll need to make some changes to the existing code.

Before you submit

  • [X] I have checked for linting errors (npm run lint) (required)
  • [ ] I have added unit tests where it makes sense to do so (encouraged but not required)
  • [ ] This change requires a documentation update (notify the documentation team)
  • [ ] This change has particular deployment requirements (notify the DevOps team)

vincentsalucci avatar Aug 25 '22 20:08 vincentsalucci