Skip to content

Conversation

@adelhult
Copy link
Contributor

No description provided.

@Nautman Nautman requested a review from Zigolox January 15, 2021 17:32
Copy link
Member

@Zigolox Zigolox left a comment

Choose a reason for hiding this comment

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

Looks good

@Nautman
Copy link
Member

Nautman commented Jan 15, 2021

Just because I know you can be peculiar, you should notice the trade-off between email being aligned and having a reasonable layout for the ProfileCards. Currently there are 4 members on the top row and 1 on the bottom. @Zigolox

@Zigolox
Copy link
Member

Zigolox commented Jan 15, 2021

Yes @Nautman, I have notices that on larger displays Karl is in a second row. If it is possible to have everyone on the same row i think it would be better.

@adelhult
Copy link
Contributor Author

adelhult commented Jan 15, 2021

Yeah, it would of been nice to fit every card on one row. Sadly it seems like the only reason it all fits with the current design is because it allows text to overflow:
overflow

If you want to avoid having one person alone on a single row, it might be a good idea to move down another profile (3 on one row and 2 on another). This can be accomplished in one of these ways:

  1. Changing to lg={4} and making all the profiles larger
  2. Nesting Grids inside of each other

@Nautman
Copy link
Member

Nautman commented Jan 15, 2021

You just reminded me that we should add a lint command to the package.json. We recommend that contributors use the ESLint extension that should be described in the README. Then the code is automatically formatted to a standard.

@Nautman
Copy link
Member

Nautman commented Jan 15, 2021

What if we moved the email to above the description? It could be below the role perhaps.

@Zigolox
Copy link
Member

Zigolox commented Jan 18, 2021

What if we moved the email to above the description? It could be below the role perhaps.

Isn't the email adress much longer than other text and would therefor have to be broken into two rows?
This migt also be a problem if the adress is further down

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.

3 participants