ParabolInc / parabol

Free online agile retrospective meeting tool
https://www.parabol.co/
Other
1.91k stars 332 forks source link

Standup UI - Responses grid #6201

Closed BartoszJarocki closed 2 years ago

BartoszJarocki commented 2 years ago

Design - https://www.figma.com/file/bhiEOV4HK9Kv91HFsJmevV/Stand-up-Concepts?node-id=1138%3A72617 Design issue - https://github.com/ParabolInc/parabol/issues/5965

Questions

AC

BartoszJarocki commented 2 years ago

@enriquesanchez @ackernaut @mattkrick I'm a bit confused about what's agreed on here. Are we doing a masonry grid or copying the same logic we use in the meeting dashboard (normal, same height grid)? i would appreciate your input!

enriquesanchez commented 2 years ago

@BartoszJarocki Sorry about that, you're right this point was not clear enough.

We decided to use the same logic devised in the /meetings route (see https://github.com/ParabolInc/parabol/issues/5965#issuecomment-1044973496)

Does this get you what you need to move forward?

BartoszJarocki commented 2 years ago

Got it, thank you @enriquesanchez! Just one more question, just to make sure we're on the same page. If we're using the same logic and UI as on meeting dashboard, it means we'll have equal height cards. Would it be possible to update the design just to make sure we know how it should look like? ie. what height each card should have etc. 🙏

enriquesanchez commented 2 years ago

it means we'll have equal height cards

@BartoszJarocki I'm not a fan of how the grid of cards looks when we have to make of all the ones in one row the same height. The reason why it was suggested to use the same logic from /meeting was that flex and grid cannot be animated.

But according to this doc from MDN it looks like they are actually animatable CSS properties.

Do you have some experience with that?

BartoszJarocki commented 2 years ago

yeah, I agree. the problem is, there's no native, and straightforward way to create a masonry layout yet. i see there's an attempt to do this natively, only supported by firefox so far: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Grid_Layout/Masonry_Layout. we use flexbox currently, and a lot of custom logic to animate cards. @mattkrick @jmtaber129 @tianrunhe, do you have any experience with creating masonry layouts?

also, what kind of animations do we want to perform in the standup meeting grid? it seems to be pretty static to me (and sorry if i missed something).

@enriquesanchez another solution worth considering is deciding what's good enough for now, so we can actually play and use the new meeting. so for example, we could do equal heights element now, next sprint, or later we can focus on making that look nicer. let me know what do you guys think.

there's one more thing that bothers me, see the screenshot below. Screenshot 2022-04-08 at 08 35 37

if we don't make items' width more variable, it's impossible (unless I'm wrong and missing something obvious) to center the whole container properly as the flex container takes all the space anyway. it just breaks my heart. any thoughts on that?

enriquesanchez commented 2 years ago

another solution worth considering is deciding what's good enough for now

This is the way. Whatever gets us to something ready for testing is the way forward. So replicating the grid behavior from /meeting is our best choice right now.

As for the heights of the response cards, this is how I see it: each card has a min-height of 100px (see mockup). A card can have a max-height of 140px, at which point we introduce scrolling. So if we follow the same logic from /meeting, then if a card in a row has a max-height of 140px, then all of the other cards in that same row should too. Does that make sense?

it's impossible (unless I'm wrong and missing something obvious) to center the whole container properly as the flex container takes all the space anyway

I believe you can center the contents of a flex container with justify-content: center?. See this guide from CSS-Tricks. Is this what you meant?

BartoszJarocki commented 2 years ago

As for the heights of the response cards, this is how I see it: each card has a min-height of 100px (see mockup). A card can have a max-height of 140px, at which point we introduce scrolling. So if we follow the same logic from /meeting, then if a card in a row has a max-height of 140px, then all of the other cards in that same row should too. Does that make sense?

yeah, but just to be clear, with the current solution the whole row will have the same height, see the screenshot below localhost_3000_meet_cnUyzNuTZu_responses (1)

@enriquesanchez are we ok with that?

I believe you can center the contents of a flex container with justify-content: center?. See this guide from CSS-Tricks. Is this what you meant?

oh yeah, I know justify-content, the problem with that is that if you center flex-row, it centers everything, see below localhost_3000_meet_cnUyzNuTZu_responses (2)

enriquesanchez commented 2 years ago

are we ok with that?

I think that for now, yes. We can stop at a max-height of 140px (and we might tweak this once we have had some time to play with it) so we don't end with very tall and mostly empty cards. How does that sound to you?

oh yeah, I know justify-content, the problem with that is that if you center flex-row, it centers everything, see below

Ah, thank you for clarifying. What if we wrap everything in another flex container? We can apply justify-content: center to the wrapper, and then use a different justify-content value for the child flex?

BartoszJarocki commented 2 years ago

I think that for now, yes. We can stop at a max-height of 140px (and we might tweak this once we have had some time to play with it) so we don't end with very tall and mostly empty cards. How does that sound to you?

sure, all clear. thank you 🙏

Ah, thank you for clarifying. What if we wrap everything in another flex container? We can apply justify-content: center to the wrapper, and then use a different justify-content value for the child flex?

unless we make card width variable (in some range) there'll be always this scenario, where the flex container (with cards) is a little bit wider than the visible parts due to wrapping the cards into the next row. it's the same situation as in the meetings dashboard. I'll try to figure out what can we do about it.

enriquesanchez commented 2 years ago

are we ok with that?

To add to this: if you have a diffeent solution or idea I'm all ears.

BartoszJarocki commented 2 years ago

To add to this: if you have a different solution or idea I'm all ears.

I believe that's the right thing to do, in the beginning. I'll do some research and talk to other folks to see if we have some other ideas for future improvements.

enriquesanchez commented 2 years ago

I'm playing with the layout on this Codepen and it's giving me more clarity on what you mean.

BartoszJarocki commented 2 years ago

@enriquesanchez yeah, exactly. If that sounds good, we can meet next week to do some real-time design/programming to make sure we're on the same page 😁

edit: @enriquesanchez I'm sorry, I just realized you're in a different team! it's just the idea, I don't want to overwork you 🙏

enriquesanchez commented 2 years ago

@BartoszJarocki Oh that'd be great! And no worries, next week I'm going to be supporting both squads, so this is perfect. Let's sync on Monday 👍