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

Use dashicons instead of images for widget icons #69

Closed
westonruter opened this Issue Dec 23, 2013 · 13 comments

Comments

Projects
None yet
3 participants
@westonruter
Contributor

westonruter commented Dec 23, 2013

See conversation at https://twitter.com/joemcasta/status/414937640970878976

The question here will be how plugins will indicate their icon. Currently we're filtering widget_icon_url so that plugins can indicate the path to a PNG image. But if we're using icon fonts, would this all be handled in CSS instead? Would a plugin just need to enqueue a stylesheet to appear on the widgets admin page and in the customizer?

See also http://core.trac.wordpress.org/ticket/25419

Builds on #58

@bordoni

This comment has been minimized.

Show comment
Hide comment
@bordoni

bordoni Dec 29, 2013

@westonruter I think we could handle all the default dashicons that are available (not so hard). It's a matter of how to identify the difference between an URL and a possible group of classes that would represent the icons.

bordoni commented Dec 29, 2013

@westonruter I think we could handle all the default dashicons that are available (not so hard). It's a matter of how to identify the difference between an URL and a possible group of classes that would represent the icons.

@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Jan 2, 2014

Contributor

@bordoni yeah, so instead of filtering widget_icon_url and rendering an image into the output of wp_widget_control(), it seems better if we just modified wp_widget_control() to add the widget's id_base as a class name (e.g. widget-{id_base} as in widget-calendar), and then the stylesheets can add the dashicon into the control where appropriate. This would allow plugins to alternatively use images instead of dashicons as well, as the stylesheets could define a background-image instead of text content.

<div id="widget-2_calendar-__i__" class="widget widget-calendar">
    <div class="widget-top">
        <div class="widget-title-action">
...

With this in mind, I don't believe there would really need to be any significant PHP to add. The only thing is adding the class name, and then the rest can be handled with CSS, and widgets can enqueue their own admin CSS to ensure the proper dashicon or background image is used.

/cc @MichaelArestad @shaunandrews

Contributor

westonruter commented Jan 2, 2014

@bordoni yeah, so instead of filtering widget_icon_url and rendering an image into the output of wp_widget_control(), it seems better if we just modified wp_widget_control() to add the widget's id_base as a class name (e.g. widget-{id_base} as in widget-calendar), and then the stylesheets can add the dashicon into the control where appropriate. This would allow plugins to alternatively use images instead of dashicons as well, as the stylesheets could define a background-image instead of text content.

<div id="widget-2_calendar-__i__" class="widget widget-calendar">
    <div class="widget-top">
        <div class="widget-title-action">
...

With this in mind, I don't believe there would really need to be any significant PHP to add. The only thing is adding the class name, and then the rest can be handled with CSS, and widgets can enqueue their own admin CSS to ensure the proper dashicon or background image is used.

/cc @MichaelArestad @shaunandrews

@MichaelArestad

This comment has been minimized.

Show comment
Hide comment
@MichaelArestad

MichaelArestad Jan 2, 2014

Contributor

@westonruter Looks good to me. It allows authors to use whatever image file type they are comfortable with.

We could get clever with CSS for default dashicons:

.widget[class*="calendar"]:before {
  content: "\f145"; /* calendar */
}
Contributor

MichaelArestad commented Jan 2, 2014

@westonruter Looks good to me. It allows authors to use whatever image file type they are comfortable with.

We could get clever with CSS for default dashicons:

.widget[class*="calendar"]:before {
  content: "\f145"; /* calendar */
}
@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Jan 2, 2014

Contributor

Great, so then we just need to have a standard location to target. How about a new element, widget-icon?

<div id="widget-2_calendar-__i__" class="widget widget-calendar">
    <div class="widget-top">
        <div class="widget-title-action">...</div>
        <div class="widget-icon"></div>
        <div class="widget-title"><h4>Calendar<span class="in-widget-title"></span></h4></div>
    </div>

The default CSS could then

.widget .widget-icon:before {
    font-family: 'dashicons';
    content: "\1234"; /* default widget icon */
}

A plugin can then override this with custom icon:

.widget-foo .widget-icon:before {
    font-family: 'foo-plugin-dashicons';
    content: "\abcd";
}

Or they could use an image instead of an icon:

.widget-foo .widget-icon:before {
    content: "";
}
.widget-foo .widget-icon {
    width: 32px;
    height: 32px;
    background-image: url( path/to/icon.svg );
}
Contributor

westonruter commented Jan 2, 2014

Great, so then we just need to have a standard location to target. How about a new element, widget-icon?

<div id="widget-2_calendar-__i__" class="widget widget-calendar">
    <div class="widget-top">
        <div class="widget-title-action">...</div>
        <div class="widget-icon"></div>
        <div class="widget-title"><h4>Calendar<span class="in-widget-title"></span></h4></div>
    </div>

The default CSS could then

.widget .widget-icon:before {
    font-family: 'dashicons';
    content: "\1234"; /* default widget icon */
}

A plugin can then override this with custom icon:

.widget-foo .widget-icon:before {
    font-family: 'foo-plugin-dashicons';
    content: "\abcd";
}

Or they could use an image instead of an icon:

.widget-foo .widget-icon:before {
    content: "";
}
.widget-foo .widget-icon {
    width: 32px;
    height: 32px;
    background-image: url( path/to/icon.svg );
}
@MichaelArestad

This comment has been minimized.

Show comment
Hide comment
@MichaelArestad

MichaelArestad Jan 2, 2014

Contributor

widget-icon makes sense.

Although, if it's an element, I would remove :before and go with:

.widget .widget-icon {
    width: 24px;
    height: 24px;
    content: "\1234"; /* default widget icon */
    font: normal 24px/1 'dashicons';
}

And again, maybe even supplement it with some logical guesses at widget names like:

.widget[class*="calendar"] .widget-icon,
.widget[class*="month"] .widget-icon,
.widget[class*="date"] .widget-icon {
    content: "\f145"; /* calendar widget icon */
}
.widget[class*="text"] .widget-icon,
.widget[class*="wysiwyg"] .widget-icon {
    content: "\f119"; /* write widget icon */
}
Contributor

MichaelArestad commented Jan 2, 2014

widget-icon makes sense.

Although, if it's an element, I would remove :before and go with:

.widget .widget-icon {
    width: 24px;
    height: 24px;
    content: "\1234"; /* default widget icon */
    font: normal 24px/1 'dashicons';
}

And again, maybe even supplement it with some logical guesses at widget names like:

.widget[class*="calendar"] .widget-icon,
.widget[class*="month"] .widget-icon,
.widget[class*="date"] .widget-icon {
    content: "\f145"; /* calendar widget icon */
}
.widget[class*="text"] .widget-icon,
.widget[class*="wysiwyg"] .widget-icon {
    content: "\f119"; /* write widget icon */
}
@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Jan 2, 2014

Contributor

I seem to remember content only working with pseudo elements (:before/:after), per the CSS2 spec:

This property is used with the :before and :after pseudo-elements to generate content in a document.

It doesn't seem to work on non-pseudo elements: http://jsfiddle.net/westonruter/R6GLH/

Contributor

westonruter commented Jan 2, 2014

I seem to remember content only working with pseudo elements (:before/:after), per the CSS2 spec:

This property is used with the :before and :after pseudo-elements to generate content in a document.

It doesn't seem to work on non-pseudo elements: http://jsfiddle.net/westonruter/R6GLH/

@MichaelArestad

This comment has been minimized.

Show comment
Hide comment
@MichaelArestad

MichaelArestad Jan 2, 2014

Contributor

Yep. You're right. Totally forgot about that.

Contributor

MichaelArestad commented Jan 2, 2014

Yep. You're right. Totally forgot about that.

@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Jan 3, 2014

Contributor

Who can take the existing PNG icons and turn them into a font which we can bundle with Widget Customizer and use for custom dashicons?

Contributor

westonruter commented Jan 3, 2014

Who can take the existing PNG icons and turn them into a font which we can bundle with Widget Customizer and use for custom dashicons?

@bordoni

This comment has been minimized.

Show comment
Hide comment
@bordoni

bordoni Jan 3, 2014

@westonruter can we grab GPL icons from other places? Or just Dashicons?

bordoni commented Jan 3, 2014

@westonruter can we grab GPL icons from other places? Or just Dashicons?

@MichaelArestad

This comment has been minimized.

Show comment
Hide comment
@MichaelArestad

MichaelArestad Jan 3, 2014

Contributor

@bordoni We technically could grab GPL icons from other places. I expect we ultimately would add to Dashicons so I would prefer we just create our own that fit nicely within the current Dashicons set.

It looks like @shaunandrews has already made a few. The ones we could add and/or make:

  • Archives
  • Custom Menu
  • Meta
  • Tag Cloud
  • Text
  • Default Widget
Contributor

MichaelArestad commented Jan 3, 2014

@bordoni We technically could grab GPL icons from other places. I expect we ultimately would add to Dashicons so I would prefer we just create our own that fit nicely within the current Dashicons set.

It looks like @shaunandrews has already made a few. The ones we could add and/or make:

  • Archives
  • Custom Menu
  • Meta
  • Tag Cloud
  • Text
  • Default Widget
@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Jan 5, 2014

Contributor

Are the ones from @shaunandrews in vector format and suitable to embed in a font?

Contributor

westonruter commented Jan 5, 2014

Are the ones from @shaunandrews in vector format and suitable to embed in a font?

@MichaelArestad

This comment has been minimized.

Show comment
Hide comment
@MichaelArestad

MichaelArestad Jan 5, 2014

Contributor

Yep. I'm embedding them in a font today.

Contributor

MichaelArestad commented Jan 5, 2014

Yep. I'm embedding them in a font today.

@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Jan 10, 2014

Contributor

Great work @MichaelArestad! Your contribution will be part of the next release (0.13), likely coming out tonight.

Contributor

westonruter commented Jan 10, 2014

Great work @MichaelArestad! Your contribution will be part of the next release (0.13), likely coming out tonight.

westonruter added a commit that referenced this issue Jan 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment