-
Notifications
You must be signed in to change notification settings - Fork 914
impress: Remove reply action from comment context menu #13534
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
base: master
Are you sure you want to change the base?
Conversation
-Replies currently merge into the parent comment body instead of creating a child comment card. -Until proper threaded replies are implemented, the reply option is removed to prevent incorrect behavior Signed-off-by: Banobe Pascal <[email protected]> Change-Id: I93e0916dec48e5f6c89f6708b18877ca1ab04983
f5f81b6 to
f7cb98e
Compare
| cy.cGet('.annotation-marker').should('not.exist'); | ||
| }); | ||
|
|
||
| it('Reply',function() { |
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.
can we add comment above why this section is skipped?
also can we add test that checks just if reply is NOT visible? so it will be obvious it is expected :)
eszkadev
left a comment
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.
code looks ok
-Since reply functionality is temporary removed, the associated tests are skipped until threaded replies are implemented. -Added a new test to explicitly assert that the "Reply" option is not visible in the context menu Signed-off-by: Banobe Pascal <[email protected]> Change-Id: Id95492a17ad0633296aedce903bd916bc052ffc9
f7cb98e to
9e8a68f
Compare
Change-Id: I2fb42256101daec6462f0472aac8941eb93953ce
Summary
TODO
Checklist
make prettier-writeand formatted the code.make checkmake runand manually verified that everything looks okay