Repository navigation
Conversation
🤖 PR meta 🤖🎉 PropsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
Updated as activity occurs, without notifying anyone named here. Add the ⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
🏁 Flaky testsSome tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information. As a user I want to be able to add multiple close buttons so that users can close the overlay from different positions in
|
|
Love these visual improvements 💖 looks much clearer |
f56fda4 to
38bede0
Compare
38bede0 to
9ab8e02
Compare
|
Thanks. Would love a ✅ |
Co-authored-by: t-hamano <54422211+t-hamano@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011g9URxVme7MjtuG8hoUPea
t-hamano
left a comment
There was a problem hiding this comment.
I fixed the unit test failures. Also, the AI pointed out an issue regarding the SVG data, but I am not sure if it is correct.
| @@ -1,3 +1,3 @@ | |||
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" style="fill: none" stroke="currentColor" stroke-width="1.5"> | |||
| <path d="M8 12V8M8 4V8M8 8H12M8 8H4M8 8L10.8284 10.8284M8 8L5.17157 5.17157M8 8L10.8284 5.17157M8 8L5.17157 10.8284M4 15H20M4 19H13" /> | |||
| <path d="M4 15H20M4 19H13M8 12V8.00001M8 8.00001V7.99999M8 8.00001L11.4641 10M8 8.00001L7.99999 8M8 7.99999V4M8 7.99999L11.4641 6M8 7.99999L7.99999 8M7.99999 8L4.53589 6M7.99999 8L4.53589 10" /> | |||
There was a problem hiding this comment.
This path contains near-zero-length segments left over from the export (V8.00001, V7.99999, L7.99999 8, etc.). Since each spoke passes through the center, it can be drawn as three straight lines with the same result:
| <path d="M4 15H20M4 19H13M8 12V8.00001M8 8.00001V7.99999M8 8.00001L11.4641 10M8 8.00001L7.99999 8M8 7.99999V4M8 7.99999L11.4641 6M8 7.99999L7.99999 8M7.99999 8L4.53589 6M7.99999 8L4.53589 10" /> | |
| <path d="M4 15H20M4 19H13M8 4V12M4.53589 6L11.4641 10M4.53589 10L11.4641 6" /> |
Also, the asterisk icon from #85442 uses stroke-linecap="square", while this one keeps the default butt caps. Is that difference intentional?
There was a problem hiding this comment.
Good question, and an aspect of refinement across the set that I'm musing on.
Endcaps means the footprint grows, sans-endcaps means only the stroke width does. In the case of the Standalone asterisk, it's intentional because it matches the optical balance of a circle, whose strokewidth would match the amount the endcap would grow. In the case of the small version, there are no endcaps on the two lines of "text" below, which means endcaps on the asterisk would imbalance things as stroke-widths changed.
I've noted a todo to ponder the endcap strategy across the set, and investigate what changes might need pushing, if any. I don't think it's a "all should have them" answer, that's part of what I need to go deep on.
Great catch, thank you. And thanks for fixing the tests.
Your other feedback is great for me to address, I'll need to dive in deep when I get a breather, thanks.
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" style="fill: none" stroke="currentColor" stroke-width="1.5"> | ||
| <path d="M4 5H20M4 9H13M17.5 13L17.3636 14.5M17.3636 14.5L17.1364 17M17.3636 14.5H15.75M17.3636 14.5H19.8636M17.1364 17L17 18.5M17.1364 17H15.5M17.1364 17H19.6364M20 13L19.8636 14.5M19.8636 14.5L19.6364 17M19.8636 14.5H21.25M19.6364 17L19.5 18.5M19.6364 17H21M6 13H12.5C13.1904 13 13.75 13.5596 13.75 14.25V18.291C13.75 18.9813 13.1904 19.541 12.5 19.541H7.76724L5.6268 21.9948C5.32306 22.343 4.75 22.1282 4.75 21.6662V19.541V14.25C4.75 13.5596 5.30964 13 6 13Z" /> | ||
| <path d="M19.5 8H13C12.3096 8 11.75 8.55964 11.75 9.25V13.291C11.75 13.9813 12.3096 14.541 13 14.541H17.7328L19.8732 16.9948C20.1769 17.343 20.75 17.1282 20.75 16.6662V14.541V9.25C20.75 8.55964 20.1904 8 19.5 8Z" /> | ||
| <path d="M4 8V10M4 10V13.5M4 10H2M4 10H7.5M4 13.5V15.5M4 13.5H2M4 13.5H7.5M7.5 8V10M7.5 10V13.5M7.5 10H9.5M7.5 13.5V15.5M7.5 13.5H9.5" /> |
There was a problem hiding this comment.
Each stroke of the number sign is split into several end-to-end segments (M4 8V10M4 10V13.5M4 13.5V15.5 …). It can be drawn as four continuous lines with the same shape, which also avoids possible anti-aliasing seams at the split points:
| <path d="M4 8V10M4 10V13.5M4 10H2M4 10H7.5M4 13.5V15.5M4 13.5H2M4 13.5H7.5M7.5 8V10M7.5 10V13.5M7.5 10H9.5M7.5 13.5V15.5M7.5 13.5H9.5" /> | |
| <path d="M4 8V15.5M7.5 8V15.5M2 10H9.5M2 13.5H9.5" /> |
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" style="fill: none" stroke="currentColor" stroke-width="1.5"> | ||
| <path d="M6.28261 7.75L6.09288 9.93182M6.09288 9.93182L5.77668 13.5682M6.09288 9.93182H3.84783M6.09288 9.93182H9.57115M5.77668 13.5682L5.58696 15.75M5.77668 13.5682H3.5M5.77668 13.5682H9.25494M9.76087 7.75L9.57115 9.93182M9.57115 9.93182L9.25494 13.5682M9.57115 9.93182H11.5M9.25494 13.5682L9.06522 15.75M9.25494 13.5682H11.1522" /> | ||
| <path d="M19 7.75C19 7.75 19 11.8033 19 13.0386C19 14.2739 19.1824 15 20.5 15M19 11.2941C19 9.23529 17.8235 8.41176 16.3824 8.41176C14.7905 8.41176 13.5 9.64706 13.5 11.7059C13.5 13.7647 14.7905 15 16.3824 15C17.5592 15 19 13.7647 19 12.1176" /> | ||
| <path d="M6 8V10M6 10V13.5M6 10H4M6 10H9.5M6 13.5V15.5M6 13.5H4M6 13.5H9.5M9.5 8V10M9.5 10V13.5M9.5 10H11.5M9.5 13.5V15.5M9.5 13.5H11.5" /> |
There was a problem hiding this comment.
Same as in post-comments-count.svg: the number sign can be drawn as four continuous lines:
| <path d="M6 8V10M6 10V13.5M6 10H4M6 10H9.5M6 13.5V15.5M6 13.5H4M6 13.5H9.5M9.5 8V10M9.5 10V13.5M9.5 10H11.5M9.5 13.5V15.5M9.5 13.5H11.5" /> | |
| <path d="M6 8V15.5M9.5 8V15.5M4 10H11.5M4 13.5H11.5" /> |
Re-export custom-post-type, word-count and post-comments-count from Figma so each stroke is one continuous line rather than several end-to-end pieces, addressing the review feedback on #85450. The asterisk loses the near-zero-length segments the export left at its centre, where the node sat at both 8.00001 and 7.99999. The two number signs go from twelve segments to four. The drawings are unchanged: word-count and post-comments-count render pixel for pixel as before, and the asterisk covers the same ink. Each line is its own path element, since Figma welds crossing strokes into a shared node when they live in one vector object. Overlapping elements are already the norm in the set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Your feedback on the points led me down a bit of a rabbit hole, but I think I managed to fix it. Thanks! |
What?
Closes #81274.
Followup to last feedback there on some of the icons for clarity and legibility, here's a simple before and after:
Observe this also updates the post type asterisk to match the one in #85442.
Noting also that I updated the recent "termCount" to the new number 2 that was larger. Previously:
Now:
CC: @poligilad-auto
Testing Instructions
npm run storybook:dev