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

[Workspace]Refactor workspace form UI #7133

Merged
merged 36 commits into from
Jul 12, 2024

Conversation

wanglam
Copy link
Contributor

@wanglam wanglam commented Jul 1, 2024

Description

This PR includes a bunch of UI updates about the workspace form. Mainly includes below changes:

  1. Use full width for workspace form in workspace create and update page
  2. Add form error callout
  3. Replace fixed bottom bar with action panel for create operation
  4. Rename "Admin" to "Owner" and add %me% to permission settings
  5. Validate at least one workspace owner in permissions settings

Issues Resolved

#7190

Screenshot

  • Create workspace
Screen.Recording.2024-07-11.at.11.37.55.mov
  • Update workspace
Screen.Recording.2024-07-11.at.11.40.49.mov

Testing the changes

  • Clone branch and run yarn osd bootstrap
  • Add below configs to config/opensearch_dashboards.yml
workspace.enabled: true
savedObjects.permission.enabled: true
  • Run yarn start --no-base-path
  • Login with admin user and goto workspace create page
  • Input form data according screenshots

Changelog

  • feat: [Workspace] Refactor workspace form UI

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Copy link
Contributor

github-actions bot commented Jul 1, 2024

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

Copy link

codecov bot commented Jul 1, 2024

Codecov Report

Attention: Patch coverage is 98.05195% with 3 lines in your changes missing coverage. Please review.

Project coverage is 67.61%. Comparing base (b422791) to head (a7b3d1d).
Report is 336 commits behind head on main.

Files with missing lines Patch % Lines
...orkspace/public/components/workspace_form/utils.ts 97.72% 0 Missing and 2 partials ⚠️
src/plugins/workspace/server/routes/index.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7133      +/-   ##
==========================================
+ Coverage   67.56%   67.61%   +0.05%     
==========================================
  Files        3469     3471       +2     
  Lines       68479    68576      +97     
  Branches    11130    11155      +25     
==========================================
+ Hits        46266    46368     +102     
+ Misses      19511    19507       -4     
+ Partials     2702     2701       -1     
Flag Coverage Δ
Linux_1 33.28% <98.05%> (+0.15%) ⬆️
Linux_2 55.26% <ø> (ø)
Linux_3 45.30% <ø> (-0.01%) ⬇️
Linux_4 34.71% <ø> (ø)
Windows_1 33.31% <98.05%> (+0.15%) ⬆️
Windows_2 55.21% <ø> (ø)
Windows_3 45.32% <ø> (ø)
Windows_4 34.71% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

SuZhou-Joe
SuZhou-Joe previously approved these changes Jul 9, 2024
ruanyl
ruanyl previously approved these changes Jul 9, 2024
});
const disabledUserOrGroupInputIdsRef = useRef(
Copy link
Member

Choose a reason for hiding this comment

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

Nit: should it use useMemo so that the computed id list will update when defaultValues.permissionSettings changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is by intentionally. The defaultValues should not be changed after workspace form component mount. It's not reflect to the form values. It's not reflect to the latest form values. We need to use these default values to disable the user or group selector .

Signed-off-by: Lin Wang <[email protected]>
@wanglam wanglam dismissed stale reviews from ruanyl and SuZhou-Joe via 2930293 July 9, 2024 05:36
@wanglam wanglam requested a review from virajsanghvi as a code owner July 9, 2024 05:36
SuZhou-Joe
SuZhou-Joe previously approved these changes Jul 9, 2024
@SuZhou-Joe SuZhou-Joe merged commit c5946b9 into opensearch-project:main Jul 12, 2024
67 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 12, 2024
* Make create workspace and update workspace full width

Signed-off-by: Lin Wang <[email protected]>

* Refactor user permissions input

Signed-off-by: Lin Wang <[email protected]>

* Add workspace form call out

Signed-off-by: Lin Wang <[email protected]>

* Fix permissions input unit tests

Signed-off-by: Lin Wang <[email protected]>

* Update gaps

Signed-off-by: Lin Wang <[email protected]>

* Update error callout

Signed-off-by: Lin Wang <[email protected]>

* Update user permission current user and number of changes

Signed-off-by: Lin Wang <[email protected]>

* Fix changes

Signed-off-by: Lin Wang <[email protected]>

* Fix owner order

Signed-off-by: Lin Wang <[email protected]>

* Add ut for form error callout

Signed-off-by: Lin Wang <[email protected]>

* Fix unit tests in workspace

Signed-off-by: Lin Wang <[email protected]>

* Mark first user row required

Signed-off-by: Lin Wang <[email protected]>

* Update section title

Signed-off-by: Lin Wang <[email protected]>

* Add validate for owner missing

Signed-off-by: Lin Wang <[email protected]>

* Changeset file for PR #7133 created/updated

* Fix unit tests for workspace form utils

Signed-off-by: Lin Wang <[email protected]>

* Fix unit tests for form error callout

Signed-off-by: Lin Wang <[email protected]>

* Add unit test for transfer current user placeholder

Signed-off-by: Lin Wang <[email protected]>

* Fix unit tests in workspace permission setting panel

Signed-off-by: Lin Wang <[email protected]>

* Fix unit test in useWorkspaceForm

Signed-off-by: Lin Wang <[email protected]>

* Add missing unit tests for workspace form utils

Signed-off-by: Lin Wang <[email protected]>

* Add unit tests for getNumberOfErrors

Signed-off-by: Lin Wang <[email protected]>

* Add more ut for workspace form error callout

Signed-off-by: Lin Wang <[email protected]>

* Fix error code

Signed-off-by: Lin Wang <[email protected]>

* Fix failed unit test

Signed-off-by: Lin Wang <[email protected]>

* Add back color picker

Signed-off-by: Lin Wang <[email protected]>

* Address UX comments

Signed-off-by: Lin Wang <[email protected]>

* Fix empty user no workspace owner

Signed-off-by: Lin Wang <[email protected]>

* Change to Associate data source

Signed-off-by: Lin Wang <[email protected]>

---------

Signed-off-by: Lin Wang <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit c5946b9)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
SuZhou-Joe pushed a commit that referenced this pull request Jul 16, 2024
* Make create workspace and update workspace full width



* Refactor user permissions input



* Add workspace form call out



* Fix permissions input unit tests



* Update gaps



* Update error callout



* Update user permission current user and number of changes



* Fix changes



* Fix owner order



* Add ut for form error callout



* Fix unit tests in workspace



* Mark first user row required



* Update section title



* Add validate for owner missing



* Changeset file for PR #7133 created/updated

* Fix unit tests for workspace form utils



* Fix unit tests for form error callout



* Add unit test for transfer current user placeholder



* Fix unit tests in workspace permission setting panel



* Fix unit test in useWorkspaceForm



* Add missing unit tests for workspace form utils



* Add unit tests for getNumberOfErrors



* Add more ut for workspace form error callout



* Fix error code



* Fix failed unit test



* Add back color picker



* Address UX comments



* Fix empty user no workspace owner



* Change to Associate data source



---------



(cherry picked from commit c5946b9)

Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
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.

4 participants