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

feat: add camelCase selectors where they were missing #4901

Merged
merged 1 commit into from
Jun 13, 2017

Conversation

jelbourn
Copy link
Member

@jelbourn jelbourn commented Jun 1, 2017

  • Also get rid of all true class bindings once and for all.
  • Also (proposing) shortening md-tab-nav-bar to just mdTabNav
  • Also (proposing) altering card image sizes (md-card-sm-image to mdCardImageSmall)

cc @andrewseguin @kara @mmalerba @tinayuangao for discussion before I do the tests/demos

After this, all of the dash-case selectors would be announced as deprecated in the changelog and removed once I can get them all out of Google apps

@jelbourn jelbourn added in progress This issue is currently in progress needs: discussion Further discussion with the team is needed before proceeding labels Jun 1, 2017
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 1, 2017
})
export class MdListSubheaderCssMatStyler {}

@Component({
moduleId: module.id,
selector: 'md-list-item, mat-list-item, a[md-list-item], a[mat-list-item]',
selector: 'md-list-item, mat-list-item, a[md-list-item], a[mat-list-item],' +
'a[mdListItem], a[matListItem]',
Copy link
Contributor

Choose a reason for hiding this comment

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

So it looks like we're just doing away with dash-case attrs entirely, even for components? (I'm happy with that, I've wanted it for a while) shouldn't we do button too?

Copy link
Member Author

Choose a reason for hiding this comment

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

This one was a mistake, reverted

@jelbourn jelbourn force-pushed the consistent-apis branch 3 times, most recently from aa90297 to 39b9add Compare June 6, 2017 19:25
@jelbourn jelbourn added pr: needs review and removed in progress This issue is currently in progress needs: discussion Further discussion with the team is needed before proceeding labels Jun 6, 2017
@jelbourn
Copy link
Member Author

jelbourn commented Jun 6, 2017

Officially for review now; I will update the tests and demos in follow-up PRs (trying to avoid merge conflicts)

host: {
'[class.mat-card-image]': 'true'
}
selector: '[md-card-lg-image], [mat-card-lg-image], [mdCarDImageLarge], [matCardImageLarge]',
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo here? Should be mdCardImageLarge

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -43,9 +42,7 @@ export class MdDialogTitle { }
*/
@Directive({
selector: '[md-dialog-content], md-dialog-content, [mat-dialog-content], mat-dialog-content',
Copy link
Contributor

Choose a reason for hiding this comment

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

Should [md-dialog-content] be updated to camelcase too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@kara kara left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Jun 6, 2017
@Input()
get mdAutosizeMinRows(): number { return this.minRows; }
set mdAutosizeMinRows(value: number) { this.minRows = value; }
@Input('matAutosizeMaxRows')
Copy link
Contributor

Choose a reason for hiding this comment

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

This is now private but relied on in internal tests. Do you want to make it deprecated for use and then make private after that?

@kara kara removed the action: merge The PR is ready for merge by the caretaker label Jun 12, 2017
@kara
Copy link
Contributor

kara commented Jun 12, 2017

@jelbourn Rebase?

@jelbourn
Copy link
Member Author

Done

@jelbourn jelbourn added action: merge The PR is ready for merge by the caretaker and removed pr: needs rebase labels Jun 12, 2017
@kara kara added the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Jun 12, 2017
@kara kara removed the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Jun 13, 2017
@kara kara merged commit 3eb0681 into angular:master Jun 13, 2017
@jelbourn jelbourn deleted the consistent-apis branch March 13, 2018 17:05
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants