-
Notifications
You must be signed in to change notification settings - Fork 36
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
DOP-4193: removed unused footer component #972
Conversation
tests/unit/FeedbackWidget.test.js
Outdated
//hideHeader={hideHeader} | ||
> | ||
<FeedbackForm /> | ||
<div> | ||
<FeedbackTab /> | ||
<Heading nodeData={headingData} sectionDepth={1} /> | ||
<FeedbackFooter /> | ||
{/* <FeedbackFooter /> */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to keep these two lines commented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the second one has been removed, see below for my reason behind keeping the above line commented
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be good with one non-blocking nit to remove a leftover comment. Thanks for working on this!
tests/unit/FeedbackWidget.test.js
Outdated
@@ -41,13 +36,12 @@ async function mountFormWithFeedbackState(feedbackState = {}, options = {}) { | |||
url: 'https://docs.mongodb.com/test', | |||
docs_property: 'test', | |||
}} | |||
hideHeader={hideHeader} | |||
//hideHeader={hideHeader} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Nit) Looks like this comment is also a leftover
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it in intentionally, as someone will probably want to add tests that the hideHeader parameter works at some point. I can remove it if you think it's unecessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I don't see any instances in which hideHeader
is actually used, and I don't think that prop currently does anything... I'm okay with keeping it for now if we want, and I can handle double-checking as part of 5-star feedback work
Stories/Links:
DOP-4193
Current Behavior:
Master Staging link
Staging Links:
Staging link
Notes:
No functionality or UI components should be changed, this PR only deletes unused code.
Some of the tests in
FeedbackWidget.test.js
were flawed or improperly described, so those were cleaned up, too.