Skip to content
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

Switch twitter by bluesky for openmicroscopy.org #764

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jburel
Copy link
Member

@jburel jburel commented Dec 12, 2024

Update link
Icons will need to be reviewed

_includes/footer.html Outdated Show resolved Hide resolved
on-the-web/index.html Outdated Show resolved Hide resolved
Copy link
Member

@pwalczysko pwalczysko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comments above - I do not think we should mislabel bluesky for twitter (i.e. do not publish twitter icons with bluesky

on-the-web/index.html Outdated Show resolved Hide resolved
Copy link
Member

@pwalczysko pwalczysko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one https://github.com/ome/www.openmicroscopy.org/pull/764/files#r1922220863 should be amended too, this is in lines with the changes you have just done in the footer ?

@pwalczysko
Copy link
Member

pwalczysko commented Jan 20, 2025

Screenshot 2025-01-20 at 12 28 04

Is the missing icon expected ?
In any case, I think we could merge after the build failure has been clarified.

Copy link
Member

@pwalczysko pwalczysko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2025-01-21 at 09 28 10

@jburel
Copy link
Member Author

jburel commented Jan 21, 2025

@sbesson @joshmoore @will-moore one of the changes in this PR is to use external fontawesome css (I have not deleted the one in this repo)

@sbesson
Copy link
Member

sbesson commented Jan 21, 2025

one of the changes in this PR is to use external fontawesome css

👍 Is the new style expected to be loaded from the CDN in production? Or should it be cached under https://github.com/ome/www.openmicroscopy.org/tree/master/css like previous FontAwesome and other styles used by the website?

@jburel
Copy link
Member Author

jburel commented Jan 21, 2025

That is the question, what is the general view about using external vs internal

@pwalczysko
Copy link
Member

That is the question, what is the general view about using external vs internal

Sorry, obviously I have approved this PR. But thiking through the discussion here, I do not like the idea that icons on the prod www.op..py.org would vanish because external server X is down (we see similar problems with the linkchecker) and thus I would lean towards caching.

@joshmoore
Copy link
Member

Definitely no objections to pulling it in if we've done that in the past, I assume following https://docs.fontawesome.com/web/setup/host-yourself/webfonts

@jburel
Copy link
Member Author

jburel commented Jan 22, 2025

https://snoopycrimecop.github.io/www.openmicroscopy.org/
Please check that no icons are missing on other pages

@pwalczysko
Copy link
Member

https://snoopycrimecop.github.io/www.openmicroscopy.org/ Please check that no icons are missing on other pages

I went through all the pages manually on snoopy.
The icons are different (see example below), but working everywhere.

New icon
Screenshot 2025-01-22 at 11 45 01
Old icon
Screenshot 2025-01-22 at 11 45 08

But lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants