close
Skip to content

Replace arrow up right html entity with icon#69572

Closed
Infinite-Null wants to merge 2 commits intoWordPress:trunkfrom
Infinite-Null:fix/add-icon-for-html-entitiy
Closed

Replace arrow up right html entity with icon#69572
Infinite-Null wants to merge 2 commits intoWordPress:trunkfrom
Infinite-Null:fix/add-icon-for-html-entitiy

Conversation

@Infinite-Null
Copy link
Contributor

@Infinite-Null Infinite-Null commented Mar 14, 2025

Fixes: #63175

What?

Replaced HTML Unicode symbol as an icon.

Screenshot:

image

@Infinite-Null Infinite-Null marked this pull request as ready for review March 17, 2025 07:26
@github-actions
Copy link

github-actions bot commented Mar 17, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Infinite-Null <ankitkumarshah@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: stokesman <presstoke@git.wordpress.org>
Co-authored-by: richtabor <richtabor@git.wordpress.org>
Co-authored-by: afercia <afercia@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@Infinite-Null Infinite-Null marked this pull request as draft March 17, 2025 07:26
@Infinite-Null Infinite-Null marked this pull request as ready for review March 17, 2025 07:33
@Infinite-Null
Copy link
Contributor Author

Hi @richtabor and @afercia,
I have changed the HTML entity with an SVG Icon please test and review it at your convenience.
Thank You!

@Mamaduka
Copy link
Member

@Infinite-Null, I think the last comment suggests the resolution path - #63175 (comment).

@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). labels Mar 19, 2025
@Infinite-Null
Copy link
Contributor Author

@Infinite-Null, I think the last comment suggests the resolution path - #63175 (comment).

Hi @Mamaduka,

Apologies, but I didn’t quite understand. Based on the comment #63175 (comment), it seems that we should replace the Unicode character with an icon, which is what I’ve implemented in this PR.

Could you please let me know if I’ve misunderstood anything? I really appreciate your help!

@stokesman
Copy link
Contributor

stokesman commented Mar 20, 2025

I think "resulting in no change visually" is the acceptance criteria though perhaps 100% parity is not required.

Here’s what I see in Chrome on macOS

Trunk This branch
external-link-trunk-chrome external-link-new-icon-chrome

I suspect that’s not close enough. There are weight, size, relative position differences and even line-height is impacted. It helps to open both in an image app (or separate tabs) to flip back and forth between them to notice all this.

While I'm sure this can be made close enough, I'm not sure it’s worth doing. I’ve read through the issue. In this component the arrow is not announced by screen readers so what is this fixing?

I definitely agree that we should fix the Site Hub’s title link. Although, there I’d favor removing the icon because the link is not external—it just opens in a new window/tab. Using the same icon feels wrong to me at least technically but maybe the meaning isn’t strictly "external" for most folks and it’s more "opens in a new tab". Also, if that’s a problem, it’s also present in the longstanding "View post" and "Preview in new tab" buttons of the editor.

@Infinite-Null
Copy link
Contributor Author

Infinite-Null commented Mar 21, 2025

While I'm sure this can be made close enough, I'm not sure it’s worth doing. I’ve read through the issue. In this component > the arrow is not announced by screen readers so what is this fixing?

Hi @stokesman,
Based on the issue description, I noticed this comment:

Also, unicode characters may be rendered in a slighly different way on different operating systems.

Considering this, I thought it would be a good improvement to replace the unicode character with an SVG to ensure consistent rendering. Let me know your thoughts!

@Infinite-Null
Copy link
Contributor Author

For now, in this PR I have removed the :: after element.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Site Hub and ExternalLink: replace the North East Arrow unicode character with equivalent SVG icon

3 participants