report: show disabled checkbox when all/no urls are third-party #9299
Conversation
There was a problem hiding this comment.
LGTM % design nit
Our design makes it kinda difficult to tell it's in a disabled state though, maybe we could at least throw cursor: not-allowed on the label? could probably use some love from @yuinchien :)
|
|
||
| // create input box | ||
| const filterTemplate = this._dom.cloneTemplate('#tmpl-lh-3p-filter', this._templateContext); | ||
| const filterInput = this._dom.find('input', filterTemplate); | ||
| const filterInput = | ||
| /** @type {HTMLInputElement} */ (this._dom.find('input', filterTemplate)); |
There was a problem hiding this comment.
man it'd really be nice to type this for some basic HTMLXElement inference
There was a problem hiding this comment.
but how often do we even select on just the tagname?
There was a problem hiding this comment.
not enough to make me want to put in the effort to do it for real :)
There was a problem hiding this comment.
LGTM2
maybe we could at least throw cursor: not-allowed on the label
how did we get cursor: pointer on there, while we're at it? That's not typical for a checkbox...
Agreed that the disabled checkbox is a little subtle since the label dominates there. Maybe remove cursor: pointer and do more to show that it's disabled?
|
|
|
I mean, looks good to me...maybe we'll issue a last call, land, and then fix later if folks have an issue with it :) |
|
Can we update the "Show 3rd Party Resources" button to the new design similar to the Performance Metrics toggle button? The product logos for Wordpress should be aligned to the top next to the text description, I might be wrong but the latest screenshot appears to have the logo center aligned horizontally. |
maybe in a follow up PR and limit this one to just keeping the option visible at all times? If we change to the toggle that makes it easy to ship this PR without further style bikeshedding, cause we aren't going to keep it anyways :) |
Fixes #9096