Skip to content
This repository has been archived by the owner on May 10, 2023. It is now read-only.

Try to apply mode styles to root html element before body #162

Merged
merged 3 commits into from
Sep 20, 2021

Conversation

daveols
Copy link
Contributor

@daveols daveols commented Sep 20, 2021

The background colour from custom modes wasn't being correctly applied to pages that had a body smaller than the view height.

@daveols daveols requested a review from jxom as a code owner September 20, 2021 02:47
@vercel
Copy link

vercel bot commented Sep 20, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

bumbag-ui – ./

🔍 Inspect: https://vercel.com/jxom/bumbag-ui/JAS1nQBZG2vUWvdx8RXS6Cj5gK5S
✅ Preview: https://bumbag-ui-git-fork-daveols-apply-mode-styles-to-root-jxom.vercel.app

@@ -4,7 +4,7 @@ import * as React from 'react';
const noflash = `(function() { try {
var mode = localStorage.getItem('bb.mode');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have a custom localStoragePrefix set personally, but this code won't work as expected for those who do.

Perhaps that prefix should be saved somewhere so that this little hack can retrieve it here and correctly apply the colour mode?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, you are right. I don't think local storage is the best solution here anyway. I think this needs more thinking & a refactor.

@jxom
Copy link
Owner

jxom commented Sep 20, 2021

Screen Shot 2021-09-20 at 1 00 48 pm

Bumbag docs seem broken in dark mode. We may need to amend the following to just 'html':
https://github.com/jxom/bumbag-ui/blob/main/packages/bumbag/src/theme/index.ts#L26

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

Successfully merging this pull request may close these issues.

2 participants