Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

[TopAppBar] Icon buttons in Card have the class mdc-icon-button while the buttons in Top App Bar don't #4527

Closed
tychenjiajun opened this issue Mar 21, 2019 · 3 comments

Comments

@tychenjiajun
Copy link

It's strange and confusing.
Card

<div class="mdc-card__action-icons">
   <button class="material-icons mdc-icon-button mdc-card__action mdc-card__action--icon" title="Share">share</button>
    <button class="material-icons mdc-icon-button mdc-card__action mdc-card__action--icon" title="More options">more_vert</button>
  </div>

Top App Bar

<section class="mdc-top-app-bar__section mdc-top-app-bar__section--align-end" role="toolbar">
      <a href="#" class="material-icons mdc-top-app-bar__action-item" aria-label="Download" alt="Download">file_download</a>
      <a href="#" class="material-icons mdc-top-app-bar__action-item" aria-label="Print this page" alt="Print this page">print</a>
      <a href="#" class="material-icons mdc-top-app-bar__action-item" aria-label="Bookmark this page" alt="Bookmark this page">bookmark</a>
    </section>
@moog16
Copy link
Contributor

moog16 commented Mar 21, 2019

@tychenjiajun the Card is just a wrapper component, which means we didn't build in icon styles. This means we have to use mdc-icon-button in the Card.

However looking at @mixin mdc-top-app-bar-icon_ is very similar to the @mixin mdc-icon-button-base_. And this might be a good opprotunity to combine the 2 mixins and use mdc-icon-button instead and remove the mdc-top-app-bar-icon mixin. I will wait for others on the team to chime in.

@moog16 moog16 added the backlog label Mar 21, 2019
@moog16
Copy link
Contributor

moog16 commented Mar 21, 2019

We will look into making top app bar support icon button, and possibly deprecate the mdc-top-app-bar-icon_ mixin in the future.

@abhiomkar abhiomkar changed the title Icon buttons in Card have the class mdc-icon-button while the buttons in Top App Bar don't [TopAppBar] Icon buttons in Card have the class mdc-icon-button while the buttons in Top App Bar don't Apr 21, 2019
@moog16 moog16 added this to the Sprint May 3rd milestone Apr 22, 2019
@moog16 moog16 self-assigned this May 8, 2019
@moog16
Copy link
Contributor

moog16 commented Jun 3, 2019

Closing. Merged #4745

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants