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

Refactor/improve workspace card #228

Merged

Conversation

tdari
Copy link
Contributor

@tdari tdari commented Feb 2, 2024

#225

  • Workspaces cards are not intuitive, the Permission: owner Status: Collaborating design is a bit confusing. We can add a divider or something to make it more clear.
    image

@tdari tdari marked this pull request as draft February 2, 2024 16:54
@tdari tdari marked this pull request as ready for review February 2, 2024 17:05
@vinicvaz vinicvaz requested a review from nathan-vm February 2, 2024 17:45
@vinicvaz
Copy link
Collaborator

vinicvaz commented Feb 2, 2024

Hey @tdari thanks for submiting this PR and nice work!

Your changes looks great! I think we can discuss about changing few things, mainly on the items position.
Here is my feedback:

  1. I really liked the Icons with tooltips, they are really nice! Maybe we can change the delete icon from filled to outlined, I think it will fit better in the card design and with the other icons, what you think?
  2. Nice to have the green border on the selected card!
  3. The first time I opened I missed an clear "instruction" to select the workspace. Maybe keeping a button for that, even being able to select directly in the card can be an option. Not sure, but we can try.
  4. The margin to the card border from the selected chip is not the same as the margin of the first button (image). Also we can try reducing just a little bit the spacing between icons to see if makes it more harmonious.
  5. Maybe (not sure if will be pretty), justify the Permission and Status to the left. My feeling looking to the card is that each one of the texts are using different alignments.

Anyway this is great and it's on the path we want, thanks again!

image

@tdari
Copy link
Contributor Author

tdari commented Feb 2, 2024

@vinicvaz thanks for the feedback. I'm going to re-submit.

@tdari tdari marked this pull request as draft February 2, 2024 22:57
@tdari tdari force-pushed the refactor/improve-workspace-card branch 2 times, most recently from 1ac27a6 to 60a71d0 Compare February 3, 2024 12:16
@tdari tdari marked this pull request as ready for review February 3, 2024 12:27
@tdari
Copy link
Contributor Author

tdari commented Feb 3, 2024

Here is the updated version. It's not pixel perfect but you can fix it if you want. I changed the text to chip to inform the user these are predefined options.

image

@tdari tdari marked this pull request as draft February 3, 2024 12:41
@tdari tdari force-pushed the refactor/improve-workspace-card branch from 60a71d0 to 06c1f58 Compare February 3, 2024 12:42
@tdari tdari marked this pull request as ready for review February 3, 2024 12:44
@vinicvaz
Copy link
Collaborator

vinicvaz commented Feb 5, 2024

@tdari great work, liked it!
I'll approve once @luiztauffer give his feedback
@luiztauffer what are your thoughts on that?

@vinicvaz vinicvaz self-requested a review February 5, 2024 16:36
@vinicvaz
Copy link
Collaborator

vinicvaz commented Feb 5, 2024

@tdari I'll merge it, looks good to me! Thanks for the contribution!

@vinicvaz vinicvaz merged commit abc2a41 into Tauffer-Consulting:dev Feb 5, 2024
@tdari
Copy link
Contributor Author

tdari commented Feb 5, 2024

You're welcome. I'm happy that we took Domino one step further 🎉

@tdari tdari deleted the refactor/improve-workspace-card branch February 5, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants