mckinsey / vizro

Vizro is a toolkit for creating modular data visualization applications.
https://vizro.readthedocs.io/en/stable/
Apache License 2.0
2.47k stars 111 forks source link

[Feat] Add CSS styling to single and multi `DatePicker` #346

Closed huong-li-nguyen closed 3 months ago

huong-li-nguyen commented 4 months ago

Description

Screenshot

Original Figma designs: Screenshot 2024-03-04 at 22 17 46

Current implementation (some deviations exist): Screenshot 2024-03-04 at 22 15 41 Screenshot 2024-03-04 at 22 15 11 Screenshot 2024-03-04 at 22 15 34 Screenshot 2024-03-04 at 22 15 04 Screenshot 2024-03-04 at 22 15 23 Screenshot 2024-03-04 at 22 14 56

Notice

huong-li-nguyen commented 4 months ago

Hey @petar-qb,

thanks for your input!

1) Should be do-able and definitely what we should do! Great catch 🚀 I've implemented it for the days already, but apparently not for the months/years. Implemented it in this commit now: https://github.com/mckinsey/vizro/pull/346/commits/ddd9658ec76bee7cc2307dc2dc1ff6ee3cbd0cb4 Screenshot 2024-03-05 at 12 29 08 Screenshot 2024-03-05 at 12 28 29

2) This would be against our design guidelines, as we don't use round edges anywhere. I know it can look nice in some cases, but for the sake of consistency, I wouldn't recommend doing that.

3) Do you mind explaining that more? Currently there is a text color / bg color differentiation between selected dates, dates in range and disabled dates. Where would you like to add more coloring? If you refer to the font color for the month, the weekdays and the date - these are intentionally the same in Figma as the main focus for the user should be on the selected day range. Hence, only the selected dates have different font colors :)

petar-qb commented 4 months ago

  1. Should be do-able and definitely what we should do! Great catch 🚀 I've implemented it for the days already, but apparently not for the months/years. Implemented it in this commit now: https://github.com/mckinsey/vizro/commit/ddd9658ec76bee7cc2307dc2dc1ff6ee3cbd0cb4

I like how the month and year texts are disabled for those outside the min-max range. Although, I can't see the difference in text color (disabled text) for days outside the min-max range. In the following screenshot, only the date 2024-07-01 is within the min-max range but it has the same text colour as all the other dates in July 2024.

image
  1. This would be agains our designed guidelines...

Makes sense.


  1. Do you mind explaining that more? Currently there is a text color / bg color differentiation between selected dates, dates in range and disabled dates. Where would you like to add more coloring? If you refer to the font color for the month, the weekdays and the date these are intentionally the same in Figma as the main focus for the user should be on the selected day range. Hence, only the selected dates have different font colors :)

Yes, I was referring to the font colour for the month, the weekdays and the date. Then it makes sense if they are intentionally made to have the same text colour.

huong-li-nguyen commented 4 months ago

Ah yes, I see! Thank you!! Fixed here: https://github.com/mckinsey/vizro/pull/346/commits/b5d1fcec66aec0438d074ff8d0e071b9aed05de0

Screenshot 2024-03-05 at 13 32 12

huong-li-nguyen commented 4 months ago

The last thing I found is that the DatePicker looks weird once it's treated as a Page.component.

image

Other than that, everything looks good 🚀

That's the expected behaviour :) The calendar will always extend to it's max-content width only while the dropdown selector takes all the entire available width (so it's consistent with the behaviour of all other selectors when added to a page). Otherwise it look off if you add more form components there. Currently users would have to update their container width for that and this will then update the width of all inner form components. This might change when we work on the form components, but for now that's the expected behaviour 👍