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

wanglam added 14 commits July 1, 2024 17:15
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
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.

Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
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 <wonglam@amazon.com>
@wanglam wanglam dismissed stale reviews from ruanyl and SuZhou-Joe via 2930293 July 9, 2024 05:36
SuZhou-Joe
SuZhou-Joe previously approved these changes Jul 9, 2024
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
@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 <wonglam@amazon.com>

* Refactor user permissions input

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Add workspace form call out

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix permissions input unit tests

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Update gaps

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Update error callout

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Update user permission current user and number of changes

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix changes

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix owner order

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Add ut for form error callout

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix unit tests in workspace

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Mark first user row required

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Update section title

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Add validate for owner missing

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Changeset file for PR #7133 created/updated

* Fix unit tests for workspace form utils

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix unit tests for form error callout

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Add unit test for transfer current user placeholder

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix unit tests in workspace permission setting panel

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix unit test in useWorkspaceForm

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Add missing unit tests for workspace form utils

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Add unit tests for getNumberOfErrors

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Add more ut for workspace form error callout

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix error code

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix failed unit test

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Add back color picker

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Address UX comments

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix empty user no workspace owner

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Change to Associate data source

Signed-off-by: Lin Wang <wonglam@amazon.com>

---------

Signed-off-by: Lin Wang <wonglam@amazon.com>
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 <wonglam@amazon.com>
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