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

Remove border conflicts with backgrounds — Update border vars #2042

Closed
bstifle opened this issue Apr 26, 2021 · 17 comments
Closed

Remove border conflicts with backgrounds — Update border vars #2042

bstifle opened this issue Apr 26, 2021 · 17 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. design Issues that need design consultation prior to development. enhancement Issues tied to a new feature or request.

Comments

@bstifle
Copy link

bstifle commented Apr 26, 2021

NEW BORDERS LIGHT

NEW BORDERS DARK

@bstifle bstifle added enhancement Issues tied to a new feature or request. 0 - new New issues that need assignment. labels Apr 26, 2021
@bstifle bstifle added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Apr 26, 2021
@macandcheese
Copy link
Contributor

Is this a calcite-colors level or tailwind-level change?

@bstifle
Copy link
Author

bstifle commented Apr 26, 2021

i think tailwind. the colors used for the new ones are existing in calcite colors

@macandcheese
Copy link
Contributor

OK, I think we actually need to update this in calcite-colors repo - the values are there: https://github.com/Esri/calcite-colors/blob/ba782b1a87deb3ef6b5e98662220f24c0e40703c/dist/colors.scss#L528

@caripizza
Copy link
Contributor

@bstifle does this need to land with v1.0.0?

@bstifle
Copy link
Author

bstifle commented May 7, 2021

Ideally yes. Im not sure if it would be a breaking change or not

@caripizza
Copy link
Contributor

@bstifle Got it, can you open an issue in that repository to get these colors updated then?

It sounds like these are the changes you're requesting right?

Sass var theme changes OLD NEW
Light theme
$ui-border-2-light: #dfdfdf #d4d4d4
$ui-border-3-light: #eaeaea #dfdfdf
Dark theme
$ui-border-1-dark: #4a4a4a #555555
$ui-border-2-dark: #404040 #4a4a4a
$ui-border-3-dark: #353535 #404040

@caripizza caripizza added 0 - new New issues that need assignment. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels May 7, 2021
@caripizza
Copy link
Contributor

@driskull @jcfranco @paulcpederson looks like we're going to need to update the Sass variables' hex values in the table above over in calcite-colors. Then, cut a new release and consume that version here in cc.

Should all issues tied to this effort (ie., in both repos) have a "breaking change" label?

@driskull
Copy link
Member

driskull commented May 7, 2021

Should all issues tied to this effort (ie., in both repos) have a "breaking change" label?

Yes, if they are breaking changes. I'm not sure the calcite-components would be breaking though.

@paulcpederson
Copy link
Member

yeah we'd have to update the theme mixins in calcite-colors to change these. Theoretically no updates on the calcite components side should be required other than updating the calcite colors lib?

@caripizza
Copy link
Contributor

Thanks @driskull and @paulcpederson! Sounds like the calcite-colors issue would want the breaking change label then, not a big difference with those values in cc.

@caripizza caripizza added design Issues that need design consultation prior to development. help wanted Issues that the core team needs help with in a sprint. labels May 13, 2021
@caripizza
Copy link
Contributor

Esri/calcite-colors#71 has been installed, just need to cut a new release. I'll work with Paul on this after next week. (Then we can use this issue to track consuming the updated version.)

@macandcheese
Copy link
Contributor

What's the status of this? Would be great to get the correct values in there, it's a little weird now when switching between dark and light theme in a UI: https://jmp.sh/k0OOfHQ

@caripizza
Copy link
Contributor

@macandcheese just waiting on a new release of calcite-colors per my comment in the linked issue cc @paulcpederson

@julio8a
Copy link

julio8a commented Aug 6, 2021

I can help publish. I'm looking at the latest for calcite-colors and it's 6.0.0, same as on NPM. Does that need to be updated before publishing a new version?

@driskull
Copy link
Member

driskull commented Aug 9, 2021

I can help publish. I'm looking at the latest for calcite-colors and it's 6.0.0, same as on NPM. Does that need to be updated before publishing a new version?

Yes.

@driskull
Copy link
Member

driskull commented Aug 9, 2021

@caripizza caripizza removed the help wanted Issues that the core team needs help with in a sprint. label Aug 9, 2021
@caripizza caripizza added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Aug 9, 2021
@caripizza caripizza self-assigned this Aug 9, 2021
@caripizza caripizza added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Aug 10, 2021
@caripizza caripizza added this to the Sprint 8/2 – 8/13 milestone Aug 10, 2021
@caripizza caripizza added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. 4 - verified Issues that have been released and confirmed resolved. and removed 2 - in development Issues that are actively being worked on. 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Aug 10, 2021
@caripizza
Copy link
Contributor

Verified on 1.0.0-next.266, the new border var values appear as:

.calcite-theme-light {
  ...
  --calcite-ui-border-2: #d4d4d4; /* (was #dfdfdf) */
  --calcite-ui-border-3: #dfdfdf; /* (was #eaeaea) */
  ...
}
  
.calcite-theme-dark {
  ...
  --calcite-ui-border-1: #555555; /* (was #4a4a4a) */
  --calcite-ui-border-2: #4a4a4a; /* (was #404040) */
  --calcite-ui-border-3: #404040; /* (was #353535) */
  ...
}  

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. design Issues that need design consultation prior to development. enhancement Issues tied to a new feature or request.
Projects
None yet
Development

No branches or pull requests

6 participants