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(code view): dark mode token updates #287

Merged
merged 71 commits into from
Jan 9, 2025
Merged

Conversation

wbarbee
Copy link
Contributor

@wbarbee wbarbee commented Dec 3, 2024

Summary

Coalesce dark and light syntax themes into a united theme that relies on new tokens with the following rules:

  1. default to the global color-scheme attribute setting for system-based dark or light mode
  2. maintain a darkTheme property that the dev can set to override the theme when desired

ADO Story or GitHub Issue Link

Code View ADO Task

Figma Link

Code View Updated Figma

To Do

  • darkTheme override hierarchy
  • check with UX on light background hex value for code view (it's not purely white)
  • delete old syntax files when done

Testing Instructions

  • currently, UX needs to update the token hex value for the light theme background:
    --kd-color-code-view-background | light-dark(#fafafc, #222527)

this will be updated as soon as available

Checklist

  • Used Conventional Commit messages as outlined in the contributing guide.
    • Noted breaking changes (if any).
  • Documented/updated all props, events, slots, parts, etc with JSDoc.
    • Ran the analyze command to update Storybook docs.
  • Added/updated Stories with controls to cover all variants.
  • Ran test locally to address any failures.
  • Added/updated the Figma link for the Story's Design tab.
  • Added any new component exports to the src/index.ts file

Screenshots

Screenshot 2024-12-09 at 10 29 40 AM Screenshot 2024-12-09 at 10 29 46 AM

Copy link

netlify bot commented Dec 3, 2024

Deploy Preview for shidoka-applications ready!

Name Link
🔨 Latest commit 1493e74
🔍 Latest deploy log https://app.netlify.com/sites/shidoka-applications/deploys/677ff1379fc98d0008b50166
😎 Deploy Preview https://deploy-preview-287--shidoka-applications.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@wbarbee wbarbee changed the title feat(code view): dark mode token updates fix(code view): dark mode token updates Dec 4, 2024
@wbarbee
Copy link
Contributor Author

wbarbee commented Dec 12, 2024

  1. I still see the copy link as a Tertiary button in Figma?
  2. Hmm we didn't actually change the syntax colors at all, did we? I assumed those were already accessible?

ok, theming should finally be good now.

for 1 above, i can convert back to the tertiary button but the styles don't match the design
Screenshot 2024-12-12 at 11 40 51 AM
Screenshot 2024-12-12 at 11 40 09 AM

for 2 (and 3), i think this will likely be resolved with the updates Robert made to the background hex value on light mode

Copy link
Contributor

@brian-patrick-3 brian-patrick-3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pending copy button design.

Copy link

@srpriyankashetty srpriyankashetty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified and looks fine in both Dark and light mode,
Please let me know if we are good with Copy button

@wbarbee wbarbee merged commit 91ac8e7 into next Jan 9, 2025
8 of 9 checks passed
@wbarbee wbarbee deleted the feat/code-view-dark-mode branch January 9, 2025 16:19
@brian-patrick-3
Copy link
Contributor

🎉 This PR is included in version 2.0.0-next.43 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants