Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

chips: remove icon is larger in 1.1.0 vs 1.0.9 #9619

Closed
ddimitrop opened this issue Sep 15, 2016 · 3 comments
Closed

chips: remove icon is larger in 1.1.0 vs 1.0.9 #9619

ddimitrop opened this issue Sep 15, 2016 · 3 comments
Assignees
Labels
- Breaking Change has: Pull Request A PR has been created to address this issue P2: required Issues that must be fixed. resolution: fixed severity: regression This issue is related to a regression type: bug type: spec alignment For issues related to aligning with the Material Design Spec
Milestone

Comments

@ddimitrop
Copy link

Actual Behavior:

  • What is the issue? *
    The remove icons for md-chips are 24px in the 1.1 version. They used to be 18px.
    The change was not intentional but due to the min-width, min-height properties that have now been added in md-icon.
    All references to md-icon in the code (where they override width and height) need to be fixed.
  • What is the expected behavior?
    The remove icons should be 18px. This is what this code is trying to do:
    https://github.com/angular/material/blob/master/src/components/chips/chips.scss#L130
    This needs to include now:
    min-height: $chip-delete-icon-size;
    min-width: $chip-delete-icon-size;

I believe that the same problem may be happening in other places like here:

width: $list-item-dense-primary-icon-width;

CodePen (or steps to reproduce the issue): *

Angular Versions: *

  • Angular Version: 1.5.5
  • `Angular Material Version: 1.1.1

Additional Information:

  • Browser Type: Chrome
  • `Browser Version: 53
  • `OS: Linux
  • Stack Traces:

Shortcut to create a new CodePen Demo.
Note: * indicates required information. Without this information, your issue may be auto-closed.

Do not modify the titles or questions. Simply add your responses to the ends of the questions.
Add more lines if needed.

@topherfangio topherfangio added - Easy fix P1: urgent Urgent issues that should be addressed in the next minor or patch release. and removed needs: triage by dev labels Jan 24, 2017
@topherfangio topherfangio added this to the 1.1.3 milestone Jan 24, 2017
@ThomasBurleson ThomasBurleson modified the milestones: 1.1.3, 1.1.4 Feb 4, 2017
@batsauto
Copy link
Contributor

here is my pull request to fix this issue.
#10491

@topherfangio topherfangio added has: Pull Request A PR has been created to address this issue needs: work labels Mar 20, 2017
@ThomasBurleson ThomasBurleson modified the milestones: 1.1.4, 1.1.5 May 7, 2017
@ThomasBurleson ThomasBurleson modified the milestones: 1.1.5, 1.1.6 Sep 5, 2017
@Splaktar Splaktar added in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs needs: work labels Nov 22, 2017
@Splaktar Splaktar added severity: regression This issue is related to a regression type: bug and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs labels Dec 4, 2017
@Splaktar
Copy link
Member

Splaktar commented Dec 4, 2017

Looking at the Material Design spec for Chips, this remove icon should not be using the Close icon as it is now. It should be using the Cancel icon instead. Also the icon should have an opacity of 0.54. And it should have some additional margins to the left and right.

This CSS seems to get as close to the spec as possible.

    min-width: 18px;
    min-height: 18px;
    position: absolute;
    margin: 0 0 0 4px;
    opacity: 0.54;

This would also require changing https://github.com/angular/material/blob/master/src/components/chips/js/chipsDirective.js#L365 to:

mdChipsCtrl.mdCloseIcon = $$mdSvgRegistry.mdCancel;

Let's try to get the icon sizing fixed in the existing PR and then tackle the rest of this in another issue/PR.

@Splaktar Splaktar modified the milestones: 1.1.6, 1.1.7 Jan 17, 2018
@Splaktar Splaktar changed the title md-chips remove icon is larger in 1.1.1 md-chips: remove icon is larger in 1.1.0 vs 1.0.9 Jan 22, 2018
@Splaktar
Copy link
Member

This breaking CSS change was part of the 1.1.0 minor release as the CSS is changed from what was in 1.0.9. As this was a minor release, breaking CSS changes like this were acceptable. Additionally, we'll need to wait for another minor release (1.2.0) before this can be fixed since it is a breaking visual change via CSS.

@Splaktar Splaktar modified the milestones: 1.1.7, Future Jan 22, 2018
@Splaktar Splaktar assigned Splaktar and unassigned ThomasBurleson Jan 22, 2018
@Splaktar Splaktar modified the milestones: Future, 1.2.0 Jan 29, 2018
@Splaktar Splaktar added P2: required Issues that must be fixed. and removed P1: urgent Urgent issues that should be addressed in the next minor or patch release. labels Feb 13, 2019
@Splaktar Splaktar changed the title md-chips: remove icon is larger in 1.1.0 vs 1.0.9 chips: remove icon is larger in 1.1.0 vs 1.0.9 Feb 13, 2019
@Splaktar Splaktar added the type: spec alignment For issues related to aligning with the Material Design Spec label Feb 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
- Breaking Change has: Pull Request A PR has been created to address this issue P2: required Issues that must be fixed. resolution: fixed severity: regression This issue is related to a regression type: bug type: spec alignment For issues related to aligning with the Material Design Spec
Projects
None yet
Development

No branches or pull requests

5 participants