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

override common bulma variables #30

Closed
wants to merge 3 commits into from

Conversation

bacongobbler
Copy link
Member

@bacongobbler bacongobbler commented Jun 13, 2022

This change remaps many of Bulma's default colors such that buttons styles with the is-primary class are now styled correctly. It is used as the basis for the Fermyon Theme for Hippo.

It also fixes the styling for error messages displayed using Bulma's message types. See the "Panels & Sections" section for a comparison.

@netlify
Copy link

netlify bot commented Jun 13, 2022

Deploy Preview for fermyon-styleguide ready!

Name Link
🔨 Latest commit 2e19469
🔍 Latest deploy log https://app.netlify.com/sites/fermyon-styleguide/deploys/62ab6f91abaf570009bb87e2
😎 Deploy Preview https://deploy-preview-30--fermyon-styleguide.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 settings.

@bacongobbler
Copy link
Member Author

It looks like newer versions of parcel minify the CSS file. I can restore it using parcel build --no-optimize, but wanted to get your opinion before going ahead with that. What do you think, @flynnduism?

@bacongobbler
Copy link
Member Author

Okay, I think I've fixed things up. Importing fermyon.css now properly styles the page.

https://design.fermyon.dev/ (prior to this change):

image

https://deploy-preview-30--fermyon-styleguide.netlify.app/ (proposed changes):

image

Do note that in both versions, switching to Dark mode does not update the color scheme for things like is-primary, making certain buttons hidden. Looking into that now.

image

@flynnduism
Copy link
Member

Looks good to me @bacongobbler, nice work!

Bulma Variable Mapping in Dark Mode

Regarding the Bulma overrides and dark mode, class mapping you're doing in fermyon-color.scss will probably need to be duplicated and adjusted for dark mode.

I.e. a second set of rules, scoped to html.dark-theme > body (see fermyon.com's example)

html.dark-theme > body {

    // Remapping Bulma Colors for dark mode
    $primary: white;
    $link: $seagreen;
    etc
}

Change to webfont embeds?

The only question I have in this PR is about the webfont CDN call being removed from the page head and moved into the stylesheet as an import.

Switching embed methods may not have an impact, but there are CDN optimizations in the <link> method (like the preconnect attribute) which seem like nice-to-haves - also moving the webfont call into the .css file will mean a slight delay in the request to fetch those assets. The webfonts are about 22kb, the css file is ~5kb so it's probably not a noticeable difference, but it won't start until after .css is fetched instead of before it.

  • Is there a reason to change how the webfont is handled?

@flynnduism flynnduism self-requested a review June 15, 2022 19:45
Copy link
Member

@flynnduism flynnduism left a comment

Choose a reason for hiding this comment

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

My comment isn't a blocker if this PR is urgent for Hippo work - as it stands this PR will improve the default appearance of Bulma UI elements, without the need for localized CSS work.

The Dark Mode color mapping can be further improved, but can be followed up on.

@bacongobbler
Copy link
Member Author

I’m currently working on fixing the dark mode issues. This is not urgent. I’ll have a closer look at your comment RE: embedding in a bit - most of this comes from Bulma’s documentation when using node-sass as the bundling mechanism

https://bulma.io/documentation/customize/with-node-sass/

@bacongobbler bacongobbler force-pushed the bulma-overrides branch 2 times, most recently from 2d7349c to 9d18667 Compare June 15, 2022 23:45
@bacongobbler
Copy link
Member Author

So I did some research into light/dark mode themes with Bulma. It looks like this has been a long-standing feature request, and it seems the maintainer(s) are unlikely going to provide support for it. jgthms/bulma#2342 has been open for 3 years.

Long-term we may want to consider switching over to TailwindCSS as it has dark mode support, and it appears to be a much more active project. But I managed to merge all of the prior work and make it work for now.

The downside is that you can't remap Bulma variables in a global class like .dark-theme. Things like this does not work:

$oxfordblue: #0D203F;

$text: $oxfordblue;

html.dark-theme > body {
    $text: white;
}

But things like this work:

$oxfordblue: #0D203F;

$text: $oxfordblue;

html.dark-theme > body {
    h1, h2, h3, h4, h5, p, li {
        color: white;
    }
}

This of course makes it very difficult to maintain a light/dark color scheme.

@bacongobbler
Copy link
Member Author

New Dark mode look:

image

@bacongobbler
Copy link
Member Author

bacongobbler commented Jun 15, 2022

The only question I have in this PR is about the webfont CDN call being removed from the page head and moved into the stylesheet as an import.

reverted. We still need to embed Bulma if we want to override those variables though.

@bacongobbler bacongobbler force-pushed the bulma-overrides branch 2 times, most recently from c51bb87 to 44dcca0 Compare June 16, 2022 00:01
Signed-off-by: Matthew Fisher <matt.fisher@fermyon.com>
Signed-off-by: Matthew Fisher <matt.fisher@fermyon.com>
@bacongobbler
Copy link
Member Author

I believe I've nailed down all of the styling issues, both in light and dark modes. @flynnduism are you okay with me merging this, or would you like to hold off until after next week? I'm fine either way :)

Signed-off-by: Matthew Fisher <matt.fisher@fermyon.com>
@bacongobbler
Copy link
Member Author

I’m going to close this for now. Not because I don’t think this work wasn’t worthwhile (quite the opposite!), but I’m concerned I’m not familiar enough how my changes will impact other sites like fermyon.com and spin.fermyon.dev. So I’m going to spike out a separate project strictly for hippo theming. Once that’s nailed down I’ll see about porting those changes over here.

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.

None yet

2 participants