Skip to content

v5: Icons!#29092

Closed
mdo wants to merge 25 commits intomasterfrom
mdo-icons-update
Closed

v5: Icons!#29092
mdo wants to merge 25 commits intomasterfrom
mdo-icons-update

Conversation

@mdo
Copy link
Copy Markdown
Member

@mdo mdo commented Jul 21, 2019 •
edited by XhmikosR
Loading

Replaces the other half of #29048.

  • Drops the Extend docs and moves the icons page to the Components section so we can have our own set
  • Adds docs for customizing and using Bootstrap Icons
  • Renames the classes to .bi
  • Optimizes (thanks @XhmikosR!)
  • Replaces the carousel icons with new compact chevrons

Still todo:

  • Tabnav between grid and table view of icons?
  • Move icons outside of content folder
  • Stop inlining the SVGs; we have http/2 so it should be pretty fast anyway?
  • Verify if we need to add role="img" or focusable="false"
  • Add a tooltip in each icon showing its title and/or class?
  • Click to copy SVG code?
  • Fix inline customize SVGs display

/cc @XhmikosR @twbs/css-review

Preview: https://deploy-preview-29092--twbs-bootstrap.netlify.com/docs/4.3/components/icons/

@mdo mdo requested a review from a team as a code owner July 21, 2019 19:35
@mdo mdo mentioned this pull request Jul 21, 2019
Closed
17 tasks
@benjaminforras
Copy link
Copy Markdown

benjaminforras commented Jul 22, 2019

Add a tooltip in each icon showing its title and/or class?

And maybe click to copy the icon's code?

@patrickhlauke
Copy link
Copy Markdown
Member

patrickhlauke commented Jul 22, 2019

yes, when the icons are actually shown, they need to somehow be presented to assistive technology/screen reader users. the <svg> itself, or its parent container, need a role="img" and an appropriate name, possibly via aria-label="..." on the element with that role.

and yes, the raw <svg> elements will need `focusable="false"

@patrickhlauke
Copy link
Copy Markdown
Member

patrickhlauke commented Jul 22, 2019

if planning to add a tooltip, the <svg> or container will need to be focusable though (so tooltip can be triggered via keyboard).

@mdo mdo force-pushed the mdo-icons-update branch from 50d5713 to 0987f88 Compare July 24, 2019 05:03
@mdo mdo force-pushed the mdo-icons-update branch from 58f0ab3 to 33a5ec8 Compare July 25, 2019 17:41
@jbalsas jbalsas mentioned this pull request Jul 31, 2019
Closed
mdo and others added 6 commits July 31, 2019 13:30
@mdo @XhmikosR
Big icon update
bda6d8b 
- Move icons to the dist folder
- Add a icons-copy npm task to bring them into the ignored directory for serving in Hugo
- Switch from inlining SVG as HTML to using img element
- Remove the icon table and only use the grid
@mdo @XhmikosR
Add more icons, add svgo npm script, change the display from dir read…
7637c11 
…ing to data yml
@XhmikosR
Don't hardcode the path.
17171af
@XhmikosR
Add missing svgo devDependency.
05767c5
@XhmikosR
Optimize inline SVGs.
d1513ba
@mdo @XhmikosR
1px strokes across the board
2608358
XhmikosR
XhmikosR reviewed Jul 31, 2019
View reviewed changes
Comment thread site/content/docs/4.3/components/icons.md Outdated
{{< listIcons.inline >}}
{{- $dirName := printf "site/static/docs/%s/dist/icons/" .Site.Params.docs_version -}}

{{- range (readDir $dirName) -}}
Copy link
Copy Markdown
Member

@XhmikosR XhmikosR Jul 31, 2019

Choose a reason for hiding this comment

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

@mdo: is the only reason for this patch the fact that you want the Bootstrap icons shown first? This adds an extra step which we could avoid. I don't mind either way, just thinking out loud.

I could probably hook something up to change the order, but it might complicate things.

@mdo mdo closed this Aug 15, 2019
@mdo mdo deleted the mdo-icons-update branch August 15, 2019 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@XhmikosR XhmikosR XhmikosR left review comments

Assignees

No one assigned

Labels

css docs v5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mdo @benjaminforras @patrickhlauke @XhmikosR