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

fix(CalendarView): Adjust Material CalendarView style #1241

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eriklimakc
Copy link
Contributor

GitHub Issue: #

PR Type

What kind of change does this PR introduce?

  • Style Update

Description

Figma Design

image

Current Control (Wasm)

image

Changes so far

image

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@eriklimakc eriklimakc self-assigned this Oct 11, 2023
@github-actions
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1241.eastus2.azurestaticapps.net

@eriklimakc
Copy link
Contributor Author

Issues

Android

When CalendarView sample page is open the Calendar doesn't appear (even without my changes)
image

General

Can't customize Today's selection since the CalendarPanel doesn't have a property to override for the border.

Figma design:
image

cc @agneszitte @kazo0 @iurycarlos

@kazo0
Copy link
Collaborator

kazo0 commented Oct 11, 2023

General

Can't customize Today's selection since the CalendarPanel doesn't have a property to override for the border.

Figma design: image

cc @agneszitte @kazo0 @iurycarlos

We have access to TodayForeground and TodayBackground, so we can change those to something that makes sense to make it stand out. But yeah I don't think we are able to customize the border surrounding the Today date @NVLudwig

@eriklimakc
Copy link
Contributor Author

eriklimakc commented Oct 11, 2023

I tried to repro the Android issue on an Uno app, but couldn't repro. Maybe the issue is only on Themes sample. I created a new issue for that #1242.

image

@kazo0 @agneszitte

@eriklimakc eriklimakc force-pushed the dev/ERLI/calendar-view-touches branch from 6ca26d4 to e740da0 Compare November 3, 2023 13:27
Copy link

github-actions bot commented Nov 3, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1241.eastus2.azurestaticapps.net

@NVLudwig
Copy link

NVLudwig commented Nov 3, 2023

This is how it renders in Safari
-Double elipses around selected items
image

@NVLudwig
Copy link

NVLudwig commented Nov 3, 2023

This is how it looks on Chrome

  • Text color of selected item varies between white and black
  • Double elipse around selected items

image

@eriklimakc eriklimakc closed this Nov 6, 2023
@eriklimakc eriklimakc reopened this Nov 6, 2023
Copy link

github-actions bot commented Nov 6, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1241.eastus2.azurestaticapps.net

@eriklimakc
Copy link
Contributor Author

eriklimakc commented Nov 6, 2023

@NVLudwig:

  • Double elipse around selected items: That is because this CalendarView has a property called SelectionMode set to Multiple.
  • Text color of selected item varies between white and black: That is because the TodayForeground is set to be the "Black" color as you can see in day 3, because it was 11/3/2023.

So @NVLudwig we can play with the following variables to make this control look better:

  • Selected Background
  • Selected Foreground
  • Today Background
  • Today Foreground
  • Today Selected Background

cc @kazo0

@iurycarlos
Copy link
Contributor

iurycarlos commented Dec 5, 2023

Hi everyone!
Could you update me this one's status? Thank you : )

@eriklimakc
Copy link
Contributor Author

Re: #1241 (comment) @iurycarlos

Hi everyone! Could you update me this one's status? Thank you : )

Figma Design

image

Changes so far

image

Can't customize Today's selection since the CalendarPanel doesn't have a property to override for the border. @NVLudwig, can you please see this comment to see what we can use to make it better #1241 (comment)?

cc @kazo0

@NVLudwig
Copy link

NVLudwig commented Dec 5, 2023

While researching the answer I found that M3 updated it's style for CalendarView so I made updates in this branch and here are the styles for: Today, Default and Selected days. (Today+Selected would be the same as Selected)

image

I did not unclude a Range styling as I can't confirm that the Win UI compoenent supports this styling (see image).

image

@eriklimakc Looking at this it feels odd that we can't have Today's date with a Border for 2 reasons:

  1. The default Selected style in Fluent has an outline (oddly Material and Fluent have the same visual styles used in reverse roles for Selected and Today's date)
    WinUICalendarView
    MaterialCalendarView

  2. We seem to have a property in Win UI that could allow us to style IsTodayHighlighted : IsTodayHighlightedProperty

I hope this helps unblock you, contact me if anything is missing

@NVLudwig
Copy link

NVLudwig commented Dec 6, 2023

@eriklimakc Here is the new UI guidance taking into account the DEV limitation uno has for borders in Calendar View.

Todays date: Fill the background with PrimaryContainerColor and fill Foreground with OnPrimaryContainerColor (see Figma for states)
Selected date or Today Selected: no change
Default dates: no change
Date range: waiting on @erik to tell me if our component supports this
image

@kazo0 I made this change because of our internal limitations. How is it that we can't use a border here when both Win UI and Material use Borders (in reversed roles)
image

@NVLudwig
Copy link

NVLudwig commented Dec 6, 2023

@eriklimakc I also noticed our Corner radius was not looking very MAterial on the modal versions so I updated it to be 28px and added a padding or 24px on all sides

Copy link

github-actions bot commented Dec 7, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1241.eastus2.azurestaticapps.net

@eriklimakc
Copy link
Contributor Author

Latest changes:

image image

@eriklimakc
Copy link
Contributor Author

@NVLudwig I didn't find any Range related property for the CalendarView control. Could @kazo0 or @Xiaoy312 confirm if such property exists?

What I see is SelectionMode that when set to Multiple behave like this:

image

@eriklimakc eriklimakc marked this pull request as ready for review December 7, 2023 19:03
@NVLudwig
Copy link

NVLudwig commented Dec 8, 2023

That's good, I still don't understand the double ellipses, is that something we have control over?

@kazo0 @iurycarlos @carldebilly Doing this exercise made me realize how much more there was to do in the UI to get closer to M3 Calendar View, nothing big but lots of little things. Here is the full change UI log it may require some updates in the Themes or Plugin side:

Figma Changes

  • Renamed Calendar Picker to Calendar View
  • Updated Calendar View Styles to fit DEV limitation: removed border on Today’s date and replaced it with PrimaryContainerColor Fill
  • Updated Corner Radius on The modal variants to 28px and added 24,24,24,16 padding
  • Added dividers between header and body
  • Updated color for dividers and borders to OutlineVariantColor
  • Updated header label:

    • Now uses sentence case
- Color = OnSurfaceVariant
- Text = Select Date
    • Changes to layer hierarchy and naming in some cases
  • Added Component Property to surface Date in the Property grid

Figma branch link

Some specs:
TopBottom
Headline
Body
Footer

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.

None yet

4 participants