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

Add css for DatePicker component #1

Open
wants to merge 34 commits into
base: dmc-15
Choose a base branch
from

Conversation

nadijagraca
Copy link

@nadijagraca nadijagraca commented Jan 16, 2025

Description

Added css for DatePicker component.

To do:

  • Add style for selected month and year

Screenshot

Notice

  • I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

    • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
    • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
    • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
    • I have not referenced individuals, products or companies in any commits, directly or indirectly.
    • I have not added data or restricted code in any commits, directly or indirectly.

nadijagraca and others added 24 commits December 9, 2024 11:30
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: huong-li-nguyen <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: joseph_perkins <[email protected]>
@huong-li-nguyen
Copy link

huong-li-nguyen commented Jan 16, 2025

Hey @nadijagraca - great job getting this done! ✨

I've added a bit more CSS as the hover states were missing. The styling of actively selected month / date doesn't seem to be supported anymore, as the component are not marked as "active". Hence, we can also add no styling for that. I don't think it's that bad though. So for me it's ready to merge 👍

I've also added a global font family for all dmc components.

@AnnMarieW
Copy link
Owner

AnnMarieW commented Jan 17, 2025

This looks nice!

It's not necessary to make updates to this unless other issues are addressed in the PR. But if this looks like it might get merged then I have just a couple comments:

  1. Would it be possible to keep the default grey color?
theme = {"primaryColor": "gray"}

This would make is to that it's a better match to the Vizro theme if people add other custom Mantine components. Otherwise those components will have the default blue accent color.

  1. Selectors that looks like this .m_396ce5cb should not be used because they are not static and can change in different versions. The static selectors are listed in the Styles API section: https://mantine.dev/dates/date-picker-input/?t=styles-api

@huong-li-nguyen
Copy link

huong-li-nguyen commented Jan 20, 2025

  1. Selectors that looks like this .m_396ce5cb should not be used because they are not static and can change in different versions. The static selectors are listed in the Styles API section: https://mantine.dev/dates/date-picker-input/?t=styles-api

Hey @AnnMarieW,

Thank you for your comments! We've now implemented all your suggestions 👍. Initially, we used selectors like .m_396ce5cb because certain media queries (used to calculate hover states) relied on these selectors and had higher specificity. As a result, the static selectors from the Styles API section weren't enough to override them.

I've now applied !important tags, which works well. It's also easier to read now 👍.

I think you would have to click on the merge button for us @AnnMarieW, as we don't have push/write access to your fork :)

@antonymilne
Copy link

This is superseded by mckinsey#959.

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.

10 participants