-
Notifications
You must be signed in to change notification settings - Fork 887
feat: add license settings UI #7210
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
Conversation
site/src/pages/DeploySettingsPage/LicensesSettingsPage/AddNewLicensePageView.tsx
Outdated
Show resolved
Hide resolved
site/src/pages/DeploySettingsPage/LicensesSettingsPage/AddNewLicensePageView.tsx
Outdated
Show resolved
Hide resolved
site/src/pages/DeploySettingsPage/LicensesSettingsPage/AddNewLicensePageView.tsx
Outdated
Show resolved
Hide resolved
site/src/pages/DeploySettingsPage/LicensesSettingsPage/AddNewLicensePageView.tsx
Outdated
Show resolved
Hide resolved
site/src/pages/DeploySettingsPage/LicensesSettingsPage/LicensesSettingsPage.tsx
Outdated
Show resolved
Hide resolved
site/src/pages/DeploySettingsPage/LicensesSettingsPage/LicensesSettingsPage.tsx
Outdated
Show resolved
Hide resolved
site/src/pages/DeploySettingsPage/LicensesSettingsPage/LicensesSettingsPage.tsx
Outdated
Show resolved
Hide resolved
site/src/pages/DeploySettingsPage/LicensesSettingsPage/LicensesSettingsPage.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Ben Potter <ben@coder.com>
I just have one minor thing, why not instead of having a button that opens a drag-and-drop area, why not display it directly there instead of the button? I also realized in the drag-and-drop area, the message is not centralized. And usually, in these kinda elements, the icon comes first and the message below is like we do in the create template page I guess. |
@BrunoQuaresma Thank you for the feedback, I indeed edited it to be a horizontal form like the one in the create template page. I need to record an updated loom video, but you can check the updated screenshots and stories in the meantime =D |
Ahh, I see. A quick video explaining what I would do to make it look a bit better. Screen.Recording.2023-04-24.at.11.58.35.mov |
site/src/pages/DeploySettingsPage/LicensesSettingsPage/LicensesSettingsPage.tsx
Outdated
Show resolved
Hide resolved
site/src/pages/DeploySettingsPage/LicensesSettingsPage/AddNewLicensePageView.tsx
Outdated
Show resolved
Hide resolved
site/src/pages/DeploySettingsPage/LicensesSettingsPage/LicensesSettingsPageView.tsx
Show resolved
Hide resolved
site/src/pages/DeploySettingsPage/LicensesSettingsPage/AddNewLicensePage.tsx
Show resolved
Hide resolved
Couple of comments but overall, very nicely done! I especially love the drag-n-drop UI and the animation. |
@rodrimaia looks pretty! |
One minor thing is when the licenses are empty looks like we are only showing the button 🤔. This is from storybook https://www.chromatic.com/test?appId=624de63c6aacee003aa84340&id=64494d5b3fb99efb4ad00fb9 |
|
Can you change it to
Also |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backend LGTM, but please add a relevant unit test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backend LGTM
https://www.loom.com/share/bb5f2f5bf8a74ae08286e2b3ff7cefdf
Updated Screenshots:
