The new icon component used a display: inline-block span in its
shadow DOM to display the icon as background of said span. The
problem is that inline elements have two heights: a line box
and a content box. The content box is correctly sized, but the
line height is dependent on other factors (some outside of the
actual component) such as font selection and line leight. This
resulted in the icon component getting a /different/ height that
what was specified. See the screenshots at here for an example:
https://imgur.com/a/reKweWU
This alignment problem was mitigated in the code-base by all kinds
of paddings and margin setting (some of which are undone in this CL).
This CL changes the span in the shadow DOM that has the actual
icon as background to a div by setting the display: block style.
This ensures that the height of the icon component is computed
as intended, and in turn, a bunch of "correction" values in a
bunch of places can be removed.
Screenshot: https://imgur.com/a/reKweWU
Bug: chromium:1126031
Change-Id: Ibe26f81817e0ef6e9564553ace070e61d75006c4
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2629550
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
- removed mentions of someIcon in the code.
- unified the source for styles and (subjective) made the data flow
more top-down.
- added support for changing the icon color via CSS as I have started
to use it and it is not always possible to change it via JS + it feels
like setting the color can be done without a rerender.
Follow-ups:
- explicit API for masked and background image components
- setting colors only via CSS?
Change-Id: I3b15dc3a1b1039a59fd5e2c5e8d2136e696f7989
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2489422
Commit-Queue: Alex Rudenko <alexrudenko@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Sigurd Schneider