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

Different styles for Up and Down arrows #4561

Closed
abdulhadi-lababidi98 opened this issue Mar 5, 2024 · 2 comments
Closed

Different styles for Up and Down arrows #4561

abdulhadi-lababidi98 opened this issue Mar 5, 2024 · 2 comments

Comments

@abdulhadi-lababidi98
Copy link

Is your feature request related to a problem? Please describe.
In multiple places the color #fff is used in the library and there is no way to control that except of overriding the the CSS rule with some selectors and !important flag, which is not the best.
Some examples:

  • src/stylesheets/mixins.scss
%triangle-arrow-up {
  fill: $datepicker__background-color;
  color: $datepicker__background-color;
  stroke: $datepicker__border-color;
}

%triangle-arrow-down {
  fill: #fff;
  color: #fff;
  stroke: $datepicker__border-color;
}

So I'm wondering here why the fill and color are not the same for both up and down arrows?

  • Also in src/stylesheets/datepicker.scss you can find multiple examples like this:
.react-datepicker {
...
  background-color: #fff;
}

Describe the solution you'd like

  • For the first example src/stylesheets/mixins.scss I suggest removing the triangle-arrow-up and triangle-arrow-down mixins since we don't really need them anymore and the arrow style now is controlled by inline CSS.
  • For the second example my suggestion would be to add a SCSS variables and not relying on inlining the colors everywhere.

Additional Context

  • After the recent update the small triangle above/below the datepicker popup changed from being a div with the after and before pseudo-elements with a lot of CSS, To a simple SVG element. So I don't think we need the triangle-arrow-[up/down] mixins anymore
@yuki0410-dev
Copy link
Contributor

yuki0410-dev commented Mar 17, 2024

@abdulhadi-lababidi98
I have created PR #4561.
I have done about removing the mixin, but not about unifying the colors of up/down, because the current situation is correct.

スクリーンショット 2024-03-17 17 14 07
スクリーンショット 2024-03-17 17 14 11

martijnrusschen added a commit that referenced this issue Mar 17, 2024
fix: #4561 remove triangle-arrow-up/down mixin
@abdulhadi-lababidi98
Copy link
Author

Okay I see the misunderstanding here, for our use case we have dark theme, so having #fff as a fixed color defeats the purpose of using the scss variables. We control the styling by doing something like this:

$datepicker__background-color: var(--color-surface-2);

.react-datepicker {
    background-color: var(--color-surface-1);
    
    &__time-container .react-datepicker__time {
        background-color: var(--color-surface-1);
    }
}

And it works perfectly when the date picker placement is on the bottom
image

but when it's on the top it switches to white (if I didn't write custom style for that)
image

And to fix the issue I had to write this custom styling:

.react-datepicker-popper[data-placement^='top'] .react-datepicker__triangle {
    fill: var(--color-surface-1);
    color: var(--color-surface-1);
}

So my suggestion is to add another scss variable called something like $datepicker__main-background-color to replace the fixed white color for the date picker
and in the src/stylesheets/datepicker.scss we can do:

.react-datepicker {
    background-color: $datepicker__main-background-color;
    
    &__time-container .react-datepicker__time {
        background-color: $datepicker__main-background-color;
    }
}

I can open a new PR in my next iteration to make that happen, but I wanna make sure that I'm not missing something here. wdyt? @yuki0410-dev

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 a pull request may close this issue.

2 participants