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.

Sign up for GitHub

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

Jump to bottom

fix(DomRenderer): correctly handle namespaced attributes #6363

Closed
wants to merge 1 commit into from
Closed

fix(DomRenderer): correctly handle namespaced attributes #6363

wants to merge 1 commit into from

Conversation

vicb
Copy link
Contributor

@vicb vicb commented Jan 8, 2016

This PR contains the fixes in #5989 rebased on #5993.

@vicb vicb added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jan 8, 2016
tbosch
tbosch reviewed Jan 21, 2016
View reviewed changes
modules/angular2/test/platform/dom/dom_renderer_integration_spec.ts
// TODO
});

describe('attributes', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you move these tests into linker/integration_spec?

Copy link
Contributor

Choose a reason for hiding this comment

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

Guard via if (DOM.supportsDOMEvents()) {

@tbosch tbosch added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 21, 2016
@vicb vicb force-pushed the 0108-atns branch 2 times, most recently from 514a26e to f32eed6 Compare January 21, 2016 18:45
@vicb vicb added the action: merge The PR is ready for merge by the caretaker label Jan 21, 2016
@rkirov
Copy link
Contributor

rkirov commented Jan 25, 2016

is this ready for merge? pls remove clean-up if so and make sure travis is green?

@alexeagle
Copy link
Contributor

Ping! remove cleanup label when you are ready to merge

@alexeagle
Copy link
Contributor

@vicb is this ready to merge?

gkalpak
gkalpak reviewed Feb 8, 2016
View reviewed changes
modules/angular2/test/core/linker/integration_spec.ts
@@ -2428,3 +2470,8 @@ class DirectiveWithPropDecorators {

fireEvent(msg) { ObservableWrapper.callEmit(this.event, msg); }
}

@Component({selector: 'some-cmp', inputs: ['value']})
Copy link
Member

Choose a reason for hiding this comment

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

Does value need to be an input ?

@vicb vicb force-pushed the 0108-atns branch 3 times, most recently from 04d853e to 5e395cf Compare February 9, 2016 17:31
@vicb vicb removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Feb 9, 2016
@vicb
Copy link
Contributor Author

vicb commented Feb 9, 2016

The PR is rebased and ready to merge

@mary-poppins
Copy link

Merging PR #6363 on behalf of @btford to branch presubmit-btford-pr-6363.

@mhevery mhevery closed this in 61cf499 Feb 10, 2016
mhevery pushed a commit that referenced this pull request Feb 10, 2016
@vicb
61cf499 fix(DomRenderer): correctly handle namespaced attributes Closes
4c8997d 
#6363
mhevery pushed a commit that referenced this pull request Feb 10, 2016
@vicb
61cf499 fix(DomRenderer): correctly handle namespaced attributes Closes
fbc9c83 
#6363
@btford btford reopened this Feb 11, 2016
@btford btford mentioned this pull request Feb 11, 2016
Closed
@btford btford assigned vicb and unassigned tbosch Feb 11, 2016
@btford btford added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker pr_state: LGTM labels Feb 11, 2016
@btford
Copy link
Contributor

btford commented Feb 11, 2016

There was an issue syncing this. Once the issue is confirmed to be resolved, I can try syncing it again.

@vicb
Copy link
Contributor Author

vicb commented Feb 12, 2016

The issue should be resolved now

@vicb vicb added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Feb 12, 2016
@mary-poppins
Copy link

Merging PR #6363 on behalf of @btford to branch presubmit-btford-pr-6363.

@btford
Copy link
Contributor

btford commented Feb 17, 2016

Thanks for your help on this, @vicb!

@matsko
Copy link
Contributor

matsko commented Feb 20, 2016

Landed as c6afea6

@matsko matsko closed this Feb 20, 2016
@vicb vicb deleted the 0108-atns branch June 23, 2016 17:43
@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.
Reviewers
No reviews
Assignees

@vicb vicb

Labels
action: merge The PR is ready for merge by the caretaker cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@vicb @rkirov @alexeagle @mary-poppins @btford @matsko @tbosch @gkalpak @googlebot