Upgrade to Lektor-Icon 3.x w/Bootstrap 5 & fix other issues #224
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
CAM-Gerlach
changed the title
CAM-Gerlach
changed the title
CAM-Gerlach
changed the title
CAM-Gerlach
reviewed
Feb 19, 2024
There was a problem hiding this comment.
A few comments so far:
- Seems like we're missing one
col-md-4in the custom CSS, and might be worth double-checking for anything else that was missed (though nothing stood out to me inspecting the remaining selectors) - The layout and sizing of the plugins gallery items has substantial differences; the live site has its three items expand to fill the full column, while in this PR they remain the same size, and additionally in this PR the second has a large bottom margin, versus zero here. I'm not seeing any custom CSS or JS for this, so we probably missed it in the theme due to not carefully checking the case of N_gallery items % 4 not equal 0 with the final version at large sizes, which differs between the Spyder and demo sites.
- Not sure how easily fixable it is here, but as a consequence of the impacts to Stellar with the theme changes, the hero graphic is scaled smaller and has much more bottom margin than before, which doesn't look as good.
- The hero image change is unrelated to the theme changes, so it should be in a separate PR (like the logo change, etc)
|
Oh yeah, I'm not done yet. |
|
Oh, oops—you'd marked this "Ready for Review", so I'd assumed it was ready sorry. |
|
I should have left it as draft... my apologies. |
- Fix regressions with css change on lektor-icon
conradolandia
force-pushed
the
css-upgrade
branch
from
0ac40ed
to
e6758fb
Compare
|
This PR is ready for review, but requires merging of lektor-icon's PR 29 to be complete. |
CAM-Gerlach
approved these changes
Mar 5, 2024
There was a problem hiding this comment.
LGTM now, everything is fixed so I think we can merge this. Thanks @conradolandia !
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 pull request aims to fix all the possible issues remaining in website-spyder after the last merge of Bootstrap 5 in lektor-icon.