refactor(icon-service): update icon reference names and registration #14598
Conversation
simeonoff
added
❌ status: awaiting-test
projects/igniteui-angular/src/lib/action-strip/grid-actions/grid-pinning-actions.component.ts
Outdated
Show resolved
Hide resolved
| @@ -20,7 +20,7 @@ | |||
| [attr.aria-labelledby]="label?.id" aria-haspopup="dialog" aria-autocomplete="none" role="combobox"> | |||
|
|
|||
| <igx-suffix *ngIf="!clearComponents.length && value" (click)="clear()"> | |||
| <igx-icon family="default" name="clear"></igx-icon> | |||
| <igx-icon family="default" name="input_clear"></igx-icon> | |||
There was a problem hiding this comment.
While I'm all for some consistency at last and I think this is the right change, just need to point out it will pull the clear icon in date and time pickers, file and filtering inputs in line with the combos.
For reference before and after, with combo below:
Could always swap it the other way around (and change the combo's), but at least it'll be consistent across the board.
There was a problem hiding this comment.
Managed to dig up this from the Simple Combo spec: https://xd.adobe.com/view/f0a2ac12-9c3d-48bd-89c8-b9f1b0ee6a25-a5f1/ so um maybe it's the Combos that are not consistent with the rest?
There was a problem hiding this comment.
I will consult with Svilen on this one, but I'd also like to keep the clear icon consistent across input, combo, select, and the various pickers we have.
There was a problem hiding this comment.
So we agreed to make changes to the date/time picker in the UI kit. From now on, they will use the same icon as the combo component.
projects/igniteui-angular/src/lib/query-builder/query-builder.component.ts
Show resolved
Hide resolved
| @@ -46,7 +46,7 @@ | |||
| }}</div> | |||
| <igx-icon | |||
| family="default" | |||
| [name]="value ? 'check' : 'close'" | |||
| [name]="value ? 'confirm' : 'close'" | |||
There was a problem hiding this comment.
This is the same ✔ and ❌ across the board, however should we sematically separate this one (for all cells). I lack the imagination to come up with alternative icons that wouldn't fit both uses, but figured should be pointed out.
There was a problem hiding this comment.
I couldn't come up with a scenario in which changing the confirm in the other places will affect the cell in any way, I believe semantically they are all the same action in all components, including the cell.
projects/igniteui-angular/src/lib/data-operations/filtering-condition.ts
Outdated
Show resolved
Hide resolved
| @@ -215,7 +215,7 @@ | |||
| (click)="toggleOperatorsDropDown($event, i)" | |||
| [igxDropDownItemNavigation]="operators" | |||
| [style.--ig-size]="filteringElementsSize"> | |||
| <igx-icon family="default" name="expand"></igx-icon> | |||
| <igx-icon family="default" name="expand_more"></igx-icon> | |||
There was a problem hiding this comment.
Would this also make sense w/ just expand?
Technically even input_expand if this https://getbootstrap.com/docs/4.0/components/dropdowns/#single-button-dropdowns is anything to go by and we're okay with a minor icon change other themes.
From what I can see the expand_more is pretty much dedicated to this button and we can drop it if we assign it one of the other two aliases.
There was a problem hiding this comment.
Then again there's the toolbar export button (for all themes):
Those two could be the same thing as well.
There was a problem hiding this comment.
Would this also make sense w/ just
expand? Technically eveninput_expandif this https://getbootstrap.com/docs/4.0/components/dropdowns/#single-button-dropdowns is anything to go by and we're okay with a minor icon change other themes. From what I can see theexpand_moreis pretty much dedicated to this button and we can drop it if we assign it one of the other two aliases.
I don't agree with input_expand as it changes from theme to theme, and is semantically different. The expand is closer in meaning, but I believe choosing the arrow_drop_down icon was intentional, thus I kept it as a separate reference used only in a few awkward places.
There was a problem hiding this comment.
They do indeed - IIRC Fluent Design consistently uses a chevron for both select and split/menu buttons, Material seems to be using this type of arrow for selects & buttons pretty consistently too, though we've strayed from that. And yes, I know that's opening another can of worms and can be dealt with separately.
There was a problem hiding this comment.
The icon in the export button of the toolbar will be changed to file_download from material.
| class="igx-pivot-data-selector__action-summary" | ||
| *ngIf="panel.type === null" | ||
| (click)="onSummaryClick($event, item, dropdown)" | ||
| [igxDropDownItemNavigation]="dropdown"> | ||
| </igx-icon> | ||
| <igx-icon | ||
| family="default" | ||
| name="drag_handle" | ||
| name="drag_indicator" |
There was a problem hiding this comment.
I personally don't mind for consistency (matches Row Drag), just noting that this used to be drag_handle which is a bit different looking:
Looks about as good or nondescript as before in the the data selector context
There was a problem hiding this comment.
I will consult with Svilen about this one. I really want to reduce the amount of ambiguous icons sprinkled all over. Technically both icons are used as drag handles.
There was a problem hiding this comment.
Svilen and I agreed that the drag_indicator will be used in both places.
| @@ -173,9 +173,9 @@ <h6 class="igx-pivot-data-selector__header-title"> | |||
| [class.igx-pivot-data-selector__item-ghost--no-drop]="!dropAllowed" | |||
| > | |||
| <div class="igx-pivot-data-selector__item-ghost-text"> | |||
| <igx-icon family="default" name="unfold_more"></igx-icon> | |||
| <igx-icon family="default" name="import_export"></igx-icon> | |||
There was a problem hiding this comment.
This one also changes the design a bit, though even before unfold_more didn't semantically sit right since it was a drag indicator on it's own trying to convey direction to move I guess? So nothing to do with the expand/collapse actions that's used elsewhere.
That said, the import/export arrows are only used in the Toolbar export button (lol on the import part) but that also doens't sound like the same thing it's here and we might need to consider this for a separate alias or something more drag related.
There was a problem hiding this comment.
Yes, it does. To be honest this icon makes a ton more sense in this scenario than the unfold_more icon as you are exporting an item from one area and importing it into another vertical area either above or below. I will consult with Svilen about this change as well. To me, the icon that should be different is the one used for the export button.
…ndition.ts Co-authored-by: Damyan Petev <damyanpetev@users.noreply.github.com>
Co-authored-by: Damyan Petev <damyanpetev@users.noreply.github.com>
Co-authored-by: Damyan Petev <damyanpetev@users.noreply.github.com>
Co-authored-by: Damyan Petev <damyanpetev@users.noreply.github.com>
tishko0
added
✅ status: verified
Closes #
Additional information (check all that apply):
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)