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

DS 653 Update icon element HTML demos #2345

Merged
merged 24 commits into from Oct 18, 2021

Conversation

MarcinMr
Copy link
Collaborator

Jira

https://pegadigitalit.atlassian.net/browse/DS-653

Summary

The Icon element HTML <SVG> demos are updated with three key attributes.

Details

The Icon element HTML <SVG> demos were updated with below attributes:

  • enable-background="new 0 0 32 32"
  • viewBox="0 0 32 32"
  • xmlns="http://www.w3.org/2000/svg"

The Icon element custom usage docs are updated.

How to test

Pull the branch. Make sure the <SVG> HTML demos have the following attributes:

  • class
  • aria-hidden
  • enable-background="new 0 0 32 32"
  • viewBox="0 0 32 32"
  • xmlns="http://www.w3.org/2000/svg"

@github-actions github-actions bot added the type: feature List this PR in the 'Features' section of the release notes. label Oct 13, 2021
Copy link
Collaborator

@danielamorse danielamorse left a comment

Choose a reason for hiding this comment

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

Nicely done @MarcinMr. I'm approving, just adding @mikemai2awesome to double-check that the updated custom SVG docs copy is accurate:

<bolt-ol>
<bolt-li>If you choose a Twig usage:
<bolt-ul>
<bolt-li>Add the element relevant attributes <code>{% verbatim %}{{ attributes.addClass(classes)|without('aria-hidden') }} aria-hidden="true"{% endverbatim %}</code> to the <code>&lt;svg&gt;</code> HTML element and save it as a Twig file. Then you can display the icon using include statement by passing the icon source path to the <code>custom_icon_path</code> prop.</bolt-li>
</bolt-ul>
</bolt-li>
<bolt-li>If you choose a HTML usage:
<bolt-ul>
<bolt-li>Add the element class <code>class="e-bolt-icon"</code> and relevant modifier classes, add the attribute <code>aria-hidden="true"</code> and the three key attributes (if they weren't provided by default): <code>enable-background="new 0 0 32 32" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg" </code> to the <code>&lt;svg&gt;</code> HTML element. Then you can display the icon using the modified <code>&lt;svg&gt;</code> markup.</bolt-li>
</bolt-ul>
</bolt-li>
</bolt-ol>
.

Copy link
Contributor

@colbytcook colbytcook left a comment

Choose a reason for hiding this comment

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

@MarcinMr I have found another couple instances that will need to be updated,

  • docs-site/src/pages/pattern-lab/_patterns/20-elements/icon/20-icon-use-case-custom-svg.twig on line 32: missing enable-background and xmlns
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-05-icon.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-10-icon-size-variation.twig on line 16: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-10-icon-theme-variation.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-15-icon-color-variation.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-25-icon-custom-colors.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-30-icon-name-variations.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/image-deprecated/-10-image-size-variations.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/image-deprecated/-15-image-source-variations.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/image-deprecated/-20-image-lazyload-variations.twig on line 4: missing aria-hidden
  • docs-site/src/templates/homepage.twig couple instances that need to be updated

@colbytcook colbytcook temporarily deployed to feature/DS-653-update-icon-element-HTML-demos--branch-preview October 15, 2021 09:17 Inactive
@colbytcook colbytcook temporarily deployed to feature/DS-653-update-icon-element-HTML-demos--branch-preview October 15, 2021 09:47 Inactive
@colbytcook colbytcook temporarily deployed to feature/DS-653-update-icon-element-HTML-demos--branch-preview October 15, 2021 10:42 Inactive
@colbytcook colbytcook temporarily deployed to feature/DS-653-update-icon-element-HTML-demos--branch-preview October 15, 2021 11:18 Inactive
@MarcinMr
Copy link
Collaborator Author

MarcinMr commented Oct 15, 2021

@MarcinMr I have found another couple instances that will need to be updated,

  • docs-site/src/pages/pattern-lab/_patterns/20-elements/icon/20-icon-use-case-custom-svg.twig on line 32: missing enable-background and xmlns
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-05-icon.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-10-icon-size-variation.twig on line 16: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-10-icon-theme-variation.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-15-icon-color-variation.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-25-icon-custom-colors.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/icon-deprecated/-30-icon-name-variations.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/image-deprecated/-10-image-size-variations.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/image-deprecated/-15-image-source-variations.twig on line 4: missing aria-hidden
  • docs-site/src/pages/pattern-lab/_patterns/40-components/image-deprecated/-20-image-lazyload-variations.twig on line 4: missing aria-hidden
  • docs-site/src/templates/homepage.twig couple instances that need to be updated

@colbytcook Thank you Colby. I updated those examples.

I also found missing enabled-background attributes in the icons and svgs directory and updated them.

@MarcinMr
Copy link
Collaborator Author

@mikemai2awesome

Regarding the docs, in the icon-use-case-custom I'm not sure about the viewBox.

For example, if the default viewBox is not 0 0 32 32 - like in our example is viewBox=0 0 496 512, then we cannot change it to another one because the icon will be zoomed in and not visible.

I'm wondering why in our docs we have an icon example with viewBox=0 0 496 512 since the first sentence in the docs says "When creating a custom SVG, start with a 32px by 32px artboard in the design tool"

Is the artboard not the same as (equivalent) the viewBox?

@mikemai2awesome
Copy link
Collaborator

@MarcinMr There are steps 1~6, if they followed that, the viewbox is already taken care of. I fixed the doc with more concise copy, and updated the icon in the demo.

@colbytcook colbytcook merged commit 1fd339c into master Oct 18, 2021
@colbytcook colbytcook deleted the feature/DS-653-update-icon-element-HTML-demos branch October 18, 2021 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature List this PR in the 'Features' section of the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants