Remove unnecessary code in GridItem's onGridCheckboxClick
Categories
(DevTools :: Inspector: Layout, enhancement, P3)
Tracking
(firefox70 fixed)
| Tracking | Status | |
|---|---|---|
| firefox70 | --- | fixed |
People
(Reporter: nchevobbe, Assigned: armando.ferreira, Mentored)
Details
(Keywords: good-first-bug)
Attachments
(2 files, 1 obsolete file)
image.png
On devtools/client/inspector/grids/components/GridItem.js#73-80, there's this piece of code:
onGridCheckboxClick(e) {
// If the click was on the svg icon to select the node in the inspector, bail out.
const originalTarget = e.nativeEvent && e.nativeEvent.explicitOriginalTarget;
if (originalTarget && originalTarget.namespaceURI === "http://www.w3.org/2000/svg") {
// We should be able to cancel the click event propagation after the following reps
// issue is implemented : https://github.com/devtools-html/reps/issues/95 .
e.preventDefault();
return;
}
The referenced issue (https://github.com/devtools-html/reps/issues/95) is resolved, which means we can get rid of this code, and call stopPropagation in onInspectIconClick
onInspectIconClick: (_, e) => {
e.stopPropagation();
this.onGridInspectIconClick(grid.nodeFront);
}
|
||
Updated•5 years ago
|
Hi, I am new to the community. I would like to work on this Issue. I made the suggested changed and was wondering if someone could offer some guidance on how to further test the changes suggested to make sure I removed the correct code and the changes didn't affect any other code.
Updated onGridCheckboxClick:
onGridCheckboxClick(e) {
const {
grid,
onToggleGridHighlighter,
} = this.props;
onToggleGridHighlighter(grid.nodeFront);
}
|
Reporter | |
Comment 2•5 years ago
|
||
Hello sohail!
To manually test this change, you can open a new tab with this as a URL:
data:text/html,<meta charset=utf8><style>html {display: grid;}</style><html>hello</html>
Then, open the inspector, select the layout pane, expand the "grid" section if it's not expanded.
You should see an checkbox with an html label, and an icon right next to it (see attachment).
Clicking on the icon shouldn't check the checkbox.
Hopefully, this is also covered by automated tests.
You can see if everything is okay by running ./mach test devtools/client/inspector/grids/test/ --headless.
Hope I was able to help you, don't hesitate to ask additional questions here or on Slack 🙂
|
|
Reporter | |
Updated•5 years ago
|
|
|
Reporter | |
Updated•5 years ago
|
|
||
Updated•5 years ago
|
|
Assignee | |
Comment 4•5 years ago
|
||
Hi Nicolas, Is this still open? Can I take it?
|
|
Reporter | |
Comment 5•5 years ago
|
||
Hello Armando, thanks for your help.
This bug is still open, yes. I assigned it to you, feel free to ask any question :)
|
|
Assignee | |
Comment 6•5 years ago
|
||
Removed unnecessary code in GridItem.js file's onGridCheckboxClick function and updated the render function so that the onInspectIconClick call triggers the stopPropagation function.
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ccc84a42f5e3 Remove unnecessary code in GridItem's onGridCheckboxClick. r=nchevobbe
|
||
Comment 8•5 years ago
|
||
| bugherder | ||
|
|
||
Updated•5 years ago
|
|
||
Updated•5 years ago
|
Description
•