-
Notifications
You must be signed in to change notification settings - Fork 65
Subscribe for updates section #1070
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
Conversation
Co-authored-by: Mia Bajić <[email protected]>
Co-authored-by: Mia Bajić <[email protected]>
for more information, see https://pre-commit.ci
Co-authored-by: Mia Bajić <[email protected]>
for more information, see https://pre-commit.ci
Co-authored-by: Mia Bajić <[email protected]>
Co-authored-by: Mia Bajić <[email protected]>
Co-authored-by: Mia Bajić <[email protected]>
Co-authored-by: Mia Bajić <[email protected]>
for more information, see https://pre-commit.ci
6e129f3
to
9385918
Compare
for more information, see https://pre-commit.ci
Preview available
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 6 out of 9 changed files in this pull request and generated no comments.
Files not reviewed (3)
- src/components/footer.astro: Language not supported
- src/components/social/SocialSubscribe.astro: Language not supported
- src/pages/index.astro: Language not supported
Feels like the social buttons are in reverse order. Don't you want to put LinkedIn first and X last? |
Thanks for the suggestion, swapped ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR is ok.
Should we prepare common solution for icons before we will use it every where?
Should we introduce right now non static sections without proper api configs and security standards.
src/components/footer.astro
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huge footer links corresponds to all previous years. If we are not introducing new design I will stay with big one.
We agreed with @nikoshell to keep the PR as a reference, but first to implement security standards and then to reimplement this section. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Changelog
Layout tests
4k
Phone
🖼️ Preview available 🖼️ : https://ep-website--1070.org.readthedocs.build/