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

Update to bootstrap 5. #844

Closed
wants to merge 2 commits into from
Closed

Update to bootstrap 5. #844

wants to merge 2 commits into from

Conversation

nojaf
Copy link
Collaborator

@nojaf nojaf commented Sep 11, 2023

This PR gives the default template a facelift with Bootstrap 5. This update makes it smoother to switch between light and dark modes, thanks to Bootstrap v5.3.

I also borrowed some ideas from Jimmy, which you can check out in this comment.

Take a peek at the new look: DarkLightFSharpFormatting

Now, I'm wondering if it's time to rethink our reliance on Bootstrap. Maybe it's more convenient to start fresh with a custom CSS theme using CSS variables for just about everything. This way, we can effortlessly handle light and dark modes from the get-go and maintain it easily. If we decide to tweak the default theme, users can do so by overriding a bunch of CSS variables.

Let's chat about this! Tagging some folks who know this repo: @dsyme, @nhirschey, @baronfel, @kMutagene, and @TheAngryByrd.

@kMutagene
Copy link
Contributor

kMutagene commented Sep 12, 2023

The dark theme looks really good!

Maybe it's more convenient to start fresh with a custom CSS theme using CSS variables for just about everything

I actually started something similar a while back on https://github.com/fslaborg/docs-template. the gist is that i used Bulma instead of bootstrap, which is a css only framework and can be customized via sass and variables. It is distributed as dotnet new template. However that approach has huge problems when the fsdocs tool receives updates.

If any css variable-based customization would find its way directly into this library, i think that would be the starting point for an actual templating system. That can be installed directly via fsdocs, e.g. a cli command fsdocs template install <name>, which then downloads a css file from an npm package. Wether you'd want to switch to bulma or do something completely from scratch does not matter too much i think.

@nhirschey
Copy link
Collaborator

I built a site with the new template and it generally looks good. I have no opinion on css vs. bootstrap.

Regarding the dark theme, I think the blue used for open/let/for/in keywords is too close to the background color. I cannot easily read these keywords in the below dark mode screenshot.

image

Copy link

@TheAngryByrd TheAngryByrd left a comment

Choose a reason for hiding this comment

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

❤️ this. Left something to consider.

}
}

toggleTheme() {

Choose a reason for hiding this comment

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

It's worth considering using prefers-color-scheme so we use whatever someone's systems defaults are.

Choose a reason for hiding this comment

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

image

I know it's not as tidy as a toggle button but I think respecting the user's system theme is more important. Some example code: code

@TheAngryByrd
Copy link

I built a site with the new template and it generally looks good. I have no opinion on css vs. bootstrap.

Regarding the dark theme, I think the blue used for open/let/for/in keywords is too close to the background color. I cannot easily read these keywords in the below dark mode screenshot.

image

I wonder if we should consider "outsourcing" code syntax to something like highlight.js (This is more future work, shouldn't block merging this tho)

@dsyme
Copy link
Contributor

dsyme commented Sep 15, 2023

I'm ok with this!

@nojaf
Copy link
Collaborator Author

nojaf commented Sep 15, 2023

Thanks all for the feedback. I'm still on the fence about using bootstrap or not.
I'm currently not using it for the new Ionide analyzers website and I think it might be reasonable to rewrite the default theme here from scratch. The CSS variable approach is working out quite well.

It could make things easier to theme, have fewer dependencies and foresee the light/dark from the start.

@nojaf
Copy link
Collaborator Author

nojaf commented Sep 30, 2023

Hi @Freymaurer

@nojaf nojaf mentioned this pull request Oct 6, 2023
@nojaf
Copy link
Collaborator Author

nojaf commented Nov 3, 2023

Closing in favour of #861

@nojaf nojaf closed this Nov 3, 2023
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.

5 participants