Support SVG icon id attributes with single quotes in the styleguide #11903
Conversation
| @@ -30,7 +30,7 @@ Draw or download an icon and save it in a template folder: | |||
|
|
|||
| The `svg` tag should: | |||
|
|
|||
| - Set the `id="icon-<name>"` attribute, icons are referenced by this name. | |||
| - Set the `id="icon-<name>"` attribute, icons are referenced by this `name`. The `name` should only contain lowercase letters, numbers, and hyphens. | |||
There was a problem hiding this comment.
Pretty sure this limitation only exists because of the styleguide's regex, and the icons can technically still be used elsewhere as long as the ID is prefixed with icon-. Alternatively we could update the regex to be more permissive.
There was a problem hiding this comment.
I think alphanumeric hyphenated should be fine? I’d be surprised if XML allowed much more.
There was a problem hiding this comment.
I think it allows at least a few others (e.g. :, ., _) and of course, uppercase letters should have no issues
https://www.w3.org/TR/REC-xml/#NT-Name
But yeah I don't think we want to encourage people to use unconventional IDs as that may cause other unexpected issues down the line. (Uppercase letters might not be so controversial though)
Manage this branch in SquashTest this branch here: https://laymonagefix-styleguide-icon-q-6wvk1.squash.io |
|
Bumping to 6.2, since at this point any new additions to 6.1 should only really be regressions and critical issues (to minimise the possibility of introducing new issues that haven't been tested with a release candidate). |
laymonage
force-pushed
the
fix-styleguide-icon-quotes
branch
from
7ee597f
to
452fe54
Compare
| @@ -30,7 +30,7 @@ Draw or download an icon and save it in a template folder: | |||
|
|
|||
| The `svg` tag should: | |||
|
|
|||
| - Set the `id="icon-<name>"` attribute, icons are referenced by this name. | |||
| - Set the `id="icon-<name>"` attribute, icons are referenced by this `name`. The `name` should only contain lowercase letters, numbers, and hyphens. | |||
There was a problem hiding this comment.
I think alphanumeric hyphenated should be fine? I’d be surprised if XML allowed much more.
wagtail/contrib/styleguide/views.py
Outdated
| # Allow no quotes, single quotes, and double quotes for the ID. | ||
| # For simplicity and readability, we don't enforce the opening | ||
| # and closing quotes to match. | ||
| icon_id_pattern = re.compile(r"""id=["']?icon-([a-z0-9-]+)["']?""") |
There was a problem hiding this comment.
I think we should drop the "no quotes" option, as it’s apparently invalid in XML – see the AttValue parsing.
There was a problem hiding this comment.
Right, I forgot SVG is XML and not HTML!
thibaudcolas
force-pushed
the
fix-styleguide-icon-quotes
branch
from
452fe54
to
4ef9909
Compare
thibaudcolas
force-pushed
the
fix-styleguide-icon-quotes
branch
from
ad16671
to
a57090f
Compare
thibaudcolas
changed the title
…11903) Co-authored-by: Thibaud Colas <thibaudcolas@gmail.com>
Fixes #11895. The HTML spec allows attributes to have no value, unquoted value, single-quoted value, and double-quoted value: https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
This issue has been around since the styleguide's icon list was reimplemented as a table in 43ca8be. In the implementation, our regex only considers the most common use case (double-quoted value).