-
Notifications
You must be signed in to change notification settings - Fork 96
Replace X with Bluesky in the site footer #1749
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2277,6 +2277,18 @@ tr, td, th { | |
background-color: #006292; | ||
color: #fff; | ||
} | ||
.btn--bluesky { | ||
background-color: #007bb6; | ||
color: #fff; | ||
} | ||
.btn--bluesky:visited { | ||
background-color: #007bb6; | ||
color: #fff; | ||
} | ||
.btn--bluesky:hover { | ||
background-color: #006292; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the preview, I'm not seeing this color change in any meaningful way when hovering over any of the footer icons. Maybe an issue to track for another PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually to be a bit more specific, I don't see these color changes in the live site either. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Hrm. I'm not sure what's going on with that. I was just mimicking the existing buttons. |
||
color: #fff; | ||
} | ||
.btn--block { | ||
display: block; | ||
width: 100%} | ||
|
@@ -3679,6 +3691,9 @@ body:hover .visually-hidden a, body:hover .visually-hidden input, body:hover .vi | |
.social-icons .fa-mastodon { | ||
color: #2b90d9; | ||
} | ||
.social-icons .fa-bluesky { | ||
color: #1185fe; | ||
} | ||
.social-icons .fa-pinterest, .social-icons .fa-pinterest-p, .social-icons .fa-pinterest-square { | ||
color: #cb2027; | ||
} | ||
|
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.
I know this isn't necessarily the right PR for this, but does anyone know why we are using these complicated svgs with path, rather than the
fa-brands fa-bluesky
, something like<i class="fab fa-bluesky" aria-hidden="true"></i>
. Do we not load the Font Awesome stylesheet at all and so the icons are not available?Uh oh!
There was an error while loading. Please reload this page.
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.
You know what, just ignore me. There isn't any reason not to really use the svg. They are in theory better since we could be handling arbitrarily sized screens. I just personally never thought about using the svgs for small icons like this, since you have to essentially go to the FA website to get the right path values, making them a little harder to work with FA icons when using them throughout a website.
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.
Ugh sorry, actually before we resolve this. I did notice we have one more file to consider. We are not using the svgs anywhere else on this website (which is fine). But more specifically, we do have a "share button" file we'll need to add fa-bluesky to, to replace twitter. _includes/buttons/share-buttons.html. I need to check to make sure how this is used, because we likely need to update the link for this as well...
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.
My interpretation, as you mentioned already, is that we do not load the whole FA stylesheet, so we use the SVGs instead. I read that loading the SVGs as we do is more performant, but I have no firsthand knowledge on the subject.
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.
I mentioned this in the Slack channel, and I was kinda thinking that it's a separate issue, since the share buttons have other things that aren't in the footer now, like Facebook and Reddit. I'm totally OK with changing them in this PR though.