zulip
zulip copied to clipboard
rendered_markdown: Fix code getting copied when visiting custom playground.
This pr fixes code getting copied even when View in xxx playground
is clicked.
Fixes: #29844.
Screenshots and screen captures:
Before | After |
---|---|
Self-review checklist
- [x] Self-reviewed the changes for clarity and maintainability (variable names, code reuse, readability, etc.).
Communicate decisions, questions, and potential concerns.
- [x] Explains differences from previous plans (e.g., issue description).
- [x] Highlights technical choices and bugs encountered.
- [x] Calls out remaining decisions and concerns.
- [x] Automated tests verify logic where appropriate.
Individual commits are ready for review (see commit discipline).
- [x] Each commit is a coherent idea.
- [x] Commit message(s) explain reasoning and motivation for changes.
Completed manual review and testing of the following:
- [x] Visual appearance of the changes.
- [x] Responsiveness and internationalization.
- [x] Strings and tooltips.
- [x] End-to-end functionality of buttons, interactions and flows.
- [x] Corner cases, error conditions, and easily imagined bugs.
I am not sure if this test (and tests like this) in rendered_markdown.test.js
needs to be changed since all the node-tests are passing.
run_test("code playground none", ({override, mock_template}) => {
override(realm_playground, "get_playground_info_for_languages", (language) => {
assert.equal(language, "javascript");
return undefined;
});
override(copied_tooltip, "show_copied_confirmation", noop);
const {prepends, $button_container, $view_code} = test_code_playground(mock_template, false);
assert.deepEqual(prepends, [$button_container]);
assert_clipboard_setup();
assert.equal($view_code.attr("data-tippy-content"), undefined);
assert.equal($view_code.attr("aria-label"), undefined);
});
@amanagr are you up for reviewing this one? I haven't tested.
LGTM
Merged, thanks @nimishmedatwal!
I guess @karlstolley FYI; this is a case where brittle JS made refactoring the HTML structure break stuff. I don't think there's an easy way to catch this sort of thing except experimentally, so I wouldn't spend undue additional time on refactoring. I don't think this type of bad pattern is common.
I guess we didn't fix the brittleness in this PR. @nimishmedatwal would you put up a follow-up that instead uses .closest()
and .find()
to find the right element without making assumptions about how many containing elements there are?
I guess we didn't fix the brittleness in this PR. @nimishmedatwal would you put up a follow-up that instead uses
.closest()
and.find()
to find the right element without making assumptions about how many containing elements there are?
Sure! I'll do that.