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

HDSG-206: Allow for Review components without an edit link #270

Merged
merged 4 commits into from
Jul 10, 2018

Conversation

pwolfert
Copy link
Collaborator

@pwolfert pwolfert commented Jul 9, 2018

Background

In App3 there are <Review> components that do not have an edit link for one reason or another. We don't need to recommend uneditable review sections as a pattern, but it would be handy for App3 if we didn't need to hack around this requirement to get the desired behavior.

Changed

  • <Review> component will not render a <ReviewLink> if the editHref prop is not provided

@@ -45,12 +59,10 @@ describe('Review', function() {

expect($p.length).toBe(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

To keep the style consistent in here, can you remove the $ from in front of the p as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was originally prefixed with $ because it was a Cheerio wrapper. In this test it is still a Cheerio wrapper, and it's still a pattern in tests in other components to prefix these Cheerio wrappers. (I took out Cheerio rendering in the others because it was unnecessary to go beyond shallow rendering to get access to our...aaaaand I just tried removing it in this test as well, and it worked, so I'll make it like the others.

Copy link
Contributor

@hannaliebl hannaliebl left a comment

Choose a reason for hiding this comment

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

One comment just related to style in test, otherwise LGTM.

@pwolfert pwolfert merged commit 8545784 into master Jul 10, 2018
@pwolfert pwolfert deleted the pwolfert/HDSG-206 branch July 10, 2018 16:42
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.

None yet

2 participants