forked from twbs/bootstrap
-
-
Notifications
You must be signed in to change notification settings - Fork 1
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
Thingies #13
Merged
julien-deramond
merged 9 commits into
julien-deramond:main-jd-bootstrap-astro
from
HiDeoo:hd-second-pass
Feb 21, 2023
Merged
Thingies #13
julien-deramond
merged 9 commits into
julien-deramond:main-jd-bootstrap-astro
from
HiDeoo:hd-second-pass
Feb 21, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This enables a markdown page with a "Examples" title to highlight the matching header link item.
The source code is now in site-new/src, dist in site-new/dist, etc.
julien-deramond
pushed a commit
that referenced
this pull request
Mar 6, 2023
* feat: pass down title to navigation component This enables a markdown page with a "Examples" title to highlight the matching header link item. * feat: port svg icons * fix: srcset syntax with embedded src * feat: add semver regular expression * feat: add home icons component content * feat: add home themes component content * refactor: move entirely astro to site-new The source code is now in site-new/src, dist in site-new/dist, etc. * feat: add (temporary) color modes * fix: opt-out explicitely of astro js bundling for external scripts
julien-deramond
pushed a commit
that referenced
this pull request
Mar 24, 2023
* feat: pass down title to navigation component This enables a markdown page with a "Examples" title to highlight the matching header link item. * feat: port svg icons * fix: srcset syntax with embedded src * feat: add semver regular expression * feat: add home icons component content * feat: add home themes component content * refactor: move entirely astro to site-new The source code is now in site-new/src, dist in site-new/dist, etc. * feat: add (temporary) color modes * fix: opt-out explicitely of astro js bundling for external scripts
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This smaller PR follows the same format as the one from yesterday, so, it's Highlightin Time!
Move site to
site-new
This feels like déjà vu, but bear with me. The previous PR introduced a move of the source of the website in
site-new
, the output of the website todist-site-new
. If we need to use the Astro public folder to serve unprocessed assets, we would have ended up with apublic-site-new
folder assite-new/public
was not an option due tosite-new
already being the sources which Astro is supposed to process. This was already becoming madness and I think my choice in the previous PR was not good as I'm too used to work in monorepos.This PR moves everything related to the new website to subfolders of
site-new
:site-new/src
contains the source of the website.site-new/dist
contains the output of the website.site-new/public
contains the public folder of the website.site-new/astro.config.mjs
contains the Astro config.This makes way more sense and things are easier to understand.
Titles
I finished the task that was still indicated as a todo and not finished from yesterday where you tried to rename the frontmatter title of the
markdown.md
page toExamples
to highlight the header link.With this PR, navigating to
http://localhost:3000/markdown
and editing the frontmatter title ofsite-new/src/pages/markdown.md
toExamples
will properly highlight the Examples link in the header.Icons
This PR ports SVG icons to Astro components. This is pretty much a copy/paste of the existing code adapted to Astro in order to make things not that different but it should be noted that it's not how icons are usually handled. Most of the time, some code or library would find svgs locally, automatically optimize them, and would serve them using a code like
<Icon name="toto" />
, e.g. a library like https://github.com/natemoo-re/astro-icon.This felt a bit too much changes compared to the current solution so I decided to not go that way for now. We can always improve this later if needed.
Color modes
We're now properly loading the color modes JavaScript file. This is a temporary solution as the file will need to be moved to a proper location in the future.
While setting up the load this JS code, I refactored all script tags in the codebase to use
is:inline
, as most of them are external scripts and this is the recommended way of doing this. By not doing this, by default, Astro will build, optimize, and add these scripts to the page automatically. There are rules where this is not the case, but I prefer to be explicit regarding loading JS code in an Astro codebase.Regarding the color modes JS code, we could technically let Astro handle this kind of code (and we will prolly let it do that for other pieces of code), altho, doing so for the color modes could potentially result in a FOUC by not inlining this code. I'm doing the exact same thing on my website.
Question
In the screenshot above (left current website, right new website), do you have any idea why the link are different (the underline). My current idea without not much investigation at all (like none at all ^^) is that this is due to the fact that this is added by the
icon-link
classes introduced in this PR (that only says added in5.3
in the docs) and that is not included in thev5.3.0-alpha1
we are using from the CDN and should be in the next alpha. If this is correct, when we switch to properly using the local version of the styles, this would be automatically fixed.Final thoughts
One of the next big thing is gonna be setting up dynamic routing, deciding to use collections or not, etc. to avoid going too far in the wrong direction. We should setup the conversation to talk about this soon.