-
Notifications
You must be signed in to change notification settings - Fork 115
Conversation
✅ Deploy Preview for bitcoin-design-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@swedishfrenchpress
swedishfrenchpress
left a comment
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.
This LGTM but I'm unsure about the docker-compose.yml changes. Holding off on explicit approval until we have another set of eyes on it. @GBKS Do you have any thoughts on those changes? Safe?
@GBKS
GBKS
left a comment
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.
Thanks for creating this PR. Some feedback:
- That some of the icons look grey is intentional, those are links you have visited before. This was introduced in #1085 based on usability feedback we received. We should keep this logic as-is. But we can remove the
.usernamerule in that CSS as it does not do anything. - Fixing the Discord logo is a good improvement. It's unnecessarily complex with the clipping path, and the the fill in the SVG breaks the hover state.
- Maybe it is time to update to the X logo, despite it being super ugly and there never being an official rebranding. Rest in peace, little blue bird.
- The BitcoinTV logo was intentionally simplified, because the original one looks ugly at that scale and does not fit in with the rest. It's been years since we posted to BitcoinTV, maybe there's a discussion to be had whether we even want to include it?
- The logos in the after screenshot are not visually balanced, some are bigger than others. Let's keep that balance.
- No docker compose updates in this PR please. That is something that affects the dev workflow and should be treated separately with proper testing by others.
Sorry, quite a bit of feedback on this seemingly simple change. Hope it all makes sense.
vivekgh0sh
commented
Jul 21, 2025
Thanks for the detailed feedback. I should've discussed first before making these changes.
- Yes I noticed the links you've visited being greyed out function now, I got confused with the discord logo and twitter logo because in the local build those link checker function were not working.
- I am thinking of resizing the discord and X logo to match the current scale of other logos.
- It's X - the everything app now.
- Agree with the BitcoinTV logo being the misfit, even now it doesn't look aligned.
- Agreed
- Yes sorry I should have asked before sending the compose changes.
Should I go ahead and make changes to the Discord and X logos?
GBKS
commented
Sep 30, 2025
Sorry for the late reply. Please go for it. Just to summarize again (if I understand correctly):
- Replace the Twitter bird with the X X
- Remove the BitcoinTV link (and SVG) altogether, since we don't post there anymore
- Fixing the Discord SVG
Thanks for contributing.
Made some improvements in footer icons as follows:
Here's a quick comparison:
Screenshot from 2025年07月20日 18-14-50
Screenshot from 2025年07月20日 18-14-42
Docker: removed the version from
docker-compose.ymlas compose file versions have been deprecated and included a --watch flag to live update changes.