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

fix(card): add camel-cased selectors to content projection #6818

Merged
merged 1 commit into from
Jan 24, 2018

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Sep 3, 2017

Fixes the camel-cased selectors for mdCardTitle and mdCardSubtitle not being added to the content projection selector.

Fixes #6816.

@jelbourn this seems along the same lines as #5647, should I get rid of the dash-cased selectors while I'm at it?

@crisbeto crisbeto requested a review from jelbourn as a code owner September 3, 2017 13:32
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 3, 2017
@julianobrasil
Copy link
Contributor

Shouldn't it also be changed in card-header.html?

@crisbeto crisbeto force-pushed the 6816/card-selectors branch from 8eb696e to 05957c1 Compare September 3, 2017 14:32
@kherock
Copy link
Contributor

kherock commented Sep 3, 2017

Can I interject with one other related question? Shouldn't md-card-header, md-card-content, md-card-actions, and md-card-footer all support camel-cased attribute selectors as well? They're all technically directives and I've been using the equivalent dialog container components using the attribute selectors for some time.

@julianobrasil
Copy link
Contributor

julianobrasil commented Sep 4, 2017

@crisbeto, once there's no @Component decorator associated, I would expected to find camelCase attributes to all of them. At least it was what I got from this: #6720 (comment) (unless there is any specificity related to md-card).

@jelbourn
Copy link
Member

jelbourn commented Sep 5, 2017

@andrewseguin where did we leave off when we last talked about these? This was one of the special cases we said we'd figure out later

@andrewseguin
Copy link
Contributor

I think we ought to sit down and look at the list of outstanding selectors we want to review to decide on how we move forward on it. Will bring it up with the team and see if we want to schedule something.

With regard to this issue, looks like this fix needs to go in regardless

Copy link
Contributor

@andrewseguin andrewseguin left a comment

Choose a reason for hiding this comment

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

Just gotta rebase

@andrewseguin andrewseguin added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Jan 22, 2018
Fixes the camel-cased selectors for `mdCardTitle` and `mdCardSubtitle` not being added to the content projection selector.

Fixes angular#6816.
@crisbeto crisbeto force-pushed the 6816/card-selectors branch from 05957c1 to b753de0 Compare January 23, 2018 18:36
@crisbeto crisbeto added the target: patch This PR is targeted for the next patch release label Jan 23, 2018
@jelbourn jelbourn merged commit d5a7cce into angular:master Jan 24, 2018
jelbourn pushed a commit to jelbourn/components that referenced this pull request Jan 29, 2018
)

Fixes the camel-cased selectors for `mdCardTitle` and `mdCardSubtitle` not being added to the content projection selector.

Fixes angular#6816.
@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 target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing mdCardHeader ng-content slot
6 participants