Skip to content
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

Adding template for igxCombo #301

Merged
merged 6 commits into from
Jul 16, 2018
Merged

Adding template for igxCombo #301

merged 6 commits into from
Jul 16, 2018

Conversation

dafo
Copy link
Collaborator

@dafo dafo commented Jul 13, 2018

Closes #298 .

this.description = "Basic IgxCombo with templating";
this.dependencies = [{
from: "igniteui-angular",
import: ["IgxComboModule", "IgxDropDownModule", "IgxCheckboxModule", "IgxToggleModule",
Copy link
Member

Choose a reason for hiding this comment

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

Those imports seem a bit excessive - I don't see IgxIcon or button being used as stand-alone cpmponenets, just the <igx-combo> and some divs. So it should be enough to import just IgxComboModule combo, right?

import { localData } from "./local-data";

@Component({
selector: 'app-$(filePrefix)',
Copy link
Member

Choose a reason for hiding this comment

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

Mixed tabs and spaces ⚠️
Should be double space as indentation for this project.


export class $(ClassName)Component implements OnInit {

public lData = localData;
Copy link
Member

Choose a reason for hiding this comment

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

You can still name that property localData if you want or you know - states or w/e.

@coveralls
Copy link

coveralls commented Jul 16, 2018

Coverage Status

Coverage remained the same at 88.532% when pulling 5ad16eb on AMarinov/addIgxComboTemplate into 4683fae on master.

@damyanpetev
Copy link
Member

@bazal4o My only issue is that now the Combo sits above Grid & List in the group and it should likely be in last place to keep the old order and match the one under https://www.infragistics.com/products/ignite-ui-angular

Thinking of merging and maybe open up an issue for the ordering as that one will require some changes to the way we load templates.

@bazal4o bazal4o merged commit 65721a8 into master Jul 16, 2018
@bazal4o bazal4o deleted the AMarinov/addIgxComboTemplate branch July 16, 2018 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants