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

chore: add svg img roles, improve title tags #2223

Merged
merged 2 commits into from
Nov 20, 2024
Merged

chore: add svg img roles, improve title tags #2223

merged 2 commits into from
Nov 20, 2024

Conversation

thecristen
Copy link
Collaborator

Summary of changes

Asana Ticket: No ticket, but noticed some <title> tags weren't a nice as they could be while using a few of these SVG files for the <.route_symbol /> component. So, this PR:

Additional optimizations because these are all inlined:

because these are all inlined:
- they SHOULD NOT have xml preambles. apparently those belong at the start of a document and there shouldn't be more than one of these in a given page (https://stackoverflow.com/questions/38169475/svg-in-html5-when-is-xml-declaration-xml-version-1-0-encoding-utf-8#comment63771563_38172170)
- xmlns/version/similar attributes on the SVG are not required
<title>
green line B
</title>
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="24px" height="24px" role="img" viewBox="0 0 24 24">
Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2024-11-18 at 10 14 06 AM

Definitely out of scope for this PR, but I would be pretty surprised if we're actually using all of icons in this folder. Maybe at some point it would be worth going through and identifying which icons we still need to keep?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep. Added to the dev discussion list!

Copy link
Contributor

@joshlarson joshlarson left a comment

Choose a reason for hiding this comment

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

Oh oops - I didn't realize I hadn't approved this already. Looks good!

@thecristen thecristen merged commit 4bf3b89 into main Nov 20, 2024
17 checks passed
@thecristen thecristen deleted the svg-enhance branch November 20, 2024 18:03
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.

2 participants