-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
URL: return early in getFilename where url argument is falsy #60265
URL: return early in getFilename where url argument is falsy #60265
Conversation
…'undefined' <string>` which could be interpreted by consumers as truthy
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 If 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. |
Size Change: +14 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
Expected: "file.jpg" Received: "[%20A%20]"
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.
Thanks for the quick follow-up! This is testing nicely for me, and I double-checked that returning undefined
for an empty string matches the behaviour on trunk, too. I also smoke tested a few different uses in the block editor and all seemed fine.
LGTM! ✨
…ss#60265) * Return early for falsy values so the URL constructor doesn't return `'undefined' <string>` which could be interpreted by consumers as truthy Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
* Trying to change the header labels for background - Background > Background image - Using the title/url of the current image in the media uploader component * Simplify copy * We don't need this wild check now that #60265 is merged * Remove placeholder image icon and center text * Fix spacing Harmonize tools panels * the background size panel displays by default in the site editor for top-level styles, but only when there's an image available. Using constant for "image" * Update copy for unset image * Update border of button to match site logo upload button Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: noisysocks <noisysocks@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: richtabor <richtabor@git.wordpress.org>
What? How? Why? When?
getFilename
should return early for falsy values so the URL constructor doesn't return'undefined' <string>
which could be interpreted by consumers as truthy. Now.First seen: https://github.com/WordPress/gutenberg/pull/60264/files#r1542177474
Before
After
Testing Instructions
Fire up the editor and run in the browser console
Unit test it.
npm run test:unit packages/url/src/test/index.js