Skip to content

adding a mutationobserver to pfe-content-set#406

Merged
kylebuch8 merged 23 commits intomasterfrom
pfe-content-set-mutationobserver
Oct 25, 2019
Merged

adding a mutationobserver to pfe-content-set#406
kylebuch8 merged 23 commits intomasterfrom
pfe-content-set-mutationobserver

Conversation

@kylebuch8
Copy link
Copy Markdown
Contributor

No description provided.

@kylebuch8 kylebuch8 requested a review from mwcz May 3, 2019 13:09
@kylebuch8 kylebuch8 added ready: branch testing Test the component from a user-perspective. Try to break it! ready: code review Ready for code review! labels May 3, 2019
Copy link
Copy Markdown
Contributor

@castastrophe castastrophe left a comment

Choose a reason for hiding this comment

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

Initial review - I noticed that in rewriting a lot of my comments were removed but they were not replaced. Can you add back a few descriptive comments?

@kylebuch8
Copy link
Copy Markdown
Contributor Author

@castastrophe In commit dbbc1d3 I added most of the comments back. I left just a few of the self explanatory comments out.

Copy link
Copy Markdown
Contributor

@mwcz mwcz left a comment

Choose a reason for hiding this comment

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

Found a small bug, not sure if caused by this PR or not. I put the full notes about it in #427.

Comment thread elements/pfe-content-set/src/pfe-content-set.js
@mwcz
Copy link
Copy Markdown
Contributor

mwcz commented Jun 17, 2019

Somehow prettier was not run on pfe-content-set.js.

@starryeyez024 starryeyez024 mentioned this pull request Sep 27, 2019
7 tasks
@starryeyez024 starryeyez024 removed ready: branch testing Test the component from a user-perspective. Try to break it! ready: code review Ready for code review! labels Sep 30, 2019
@starryeyez024 starryeyez024 added the needs code updates Code updates have been requested. label Sep 30, 2019
Comment thread elements/pfe-content-set/src/pfe-content-set.js
castastrophe
castastrophe previously approved these changes Oct 25, 2019
Copy link
Copy Markdown
Contributor

@castastrophe castastrophe left a comment

Choose a reason for hiding this comment

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

👍

@kylebuch8 kylebuch8 removed the needs code updates Code updates have been requested. label Oct 25, 2019
mwcz
mwcz previously approved these changes Oct 25, 2019
Copy link
Copy Markdown
Contributor

@mwcz mwcz left a comment

Choose a reason for hiding this comment

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

Look! Grotesque Tabs Mutation!

Copy link
Copy Markdown
Contributor

@mwcz mwcz left a comment

Choose a reason for hiding this comment

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

Licensing GuaranteeTM

@kylebuch8 kylebuch8 merged commit 732ca3e into master Oct 25, 2019
@kylebuch8 kylebuch8 deleted the pfe-content-set-mutationobserver branch October 25, 2019 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants