Skip to content

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

Merged
merged 22 commits into from
Apr 26, 2023
Merged

feat: add license settings UI #7210

merged 22 commits into from
Apr 26, 2023

Conversation

rodrimaia
Copy link
Contributor

@rodrimaia rodrimaia commented Apr 19, 2023

@rodrimaia rodrimaia self-assigned this Apr 19, 2023
rodrimaia and others added 2 commits April 21, 2023 10:28
Co-authored-by: Ben Potter <ben@coder.com>
@rodrimaia rodrimaia changed the title Add license settings UI feat: add license settings UI Apr 21, 2023
@rodrimaia rodrimaia marked this pull request as ready for review April 24, 2023 14:46
@rodrimaia rodrimaia requested review from a team, BrunoQuaresma and bpmct and removed request for a team April 24, 2023 14:46
@BrunoQuaresma
Copy link
Collaborator

BrunoQuaresma commented Apr 24, 2023

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.

@rodrimaia
Copy link
Contributor Author

rodrimaia commented Apr 24, 2023

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

@BrunoQuaresma
Copy link
Collaborator

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

@Kira-Pilot
Copy link
Member

Couple of comments but overall, very nicely done! I especially love the drag-n-drop UI and the animation.

@rodrimaia
Copy link
Contributor Author

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

image
Fixed! Thank you for suggesting

@BrunoQuaresma
Copy link
Collaborator

@rodrimaia looks pretty!

@BrunoQuaresma
Copy link
Collaborator

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

Screen Shot 2023-04-26 at 13 25 37

@rodrimaia
Copy link
Contributor Author

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

Screen Shot 2023-04-26 at 13 25 37

image
fixed! :)

@bpmct
Copy link
Member

bpmct commented Apr 26, 2023

Screenshot 2023-04-26 at 1 18 31 PM

Can you change it to

Contact sales or request a trial license to learn more

Also No Licenses yet -> No licenses yet

@rodrimaia
Copy link
Contributor Author

Fixed! thanks Ben
image

Copy link
Member

@mtojek mtojek left a 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.

@mtojek mtojek self-requested a review April 26, 2023 20:03
Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

Backend LGTM

@rodrimaia rodrimaia merged commit 87b7537 into main Apr 26, 2023
@rodrimaia rodrimaia deleted the add_license_settings branch April 26, 2023 20:47
@github-actions github-actions bot locked and limited conversation to collaborators Apr 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants