TintableSvgButton styling #5605
Conversation
| @@ -332,3 +332,30 @@ textarea { | |||
| border: 1px solid gray; | |||
| background-color: darkred; | |||
| } | |||
|
|
|||
| .mx_TintableSvgButton { | |||
There was a problem hiding this comment.
I'm very surprised that the TintableSvgButton CSS was wrong, and i'm failing to see why this is needed? What's the story? :)
There was a problem hiding this comment.
oh, having read the react-sdk stuff this makes a bit more sense. thinks
There was a problem hiding this comment.
okay, i'm still a bit confused - is this just because you can't put alt text on an <object/>? surely cursor: pointer could work on an <object/>?
There was a problem hiding this comment.
Yup, it's mainly because you can't put alt text on the svg. The path element of the svg also overrides cursor styling of the object / svg.
We could do the styling overrides in custom CSS. However, it seems like a fairly standard occurrence that we would want tintable SVGs to behave in the same way as standard "img" elements, so why not just add it here, along with the title text overlay?
There was a problem hiding this comment.
ok, lgtm. still dubious about having two spans introduced though (as per other PR)
There was a problem hiding this comment.
"surely cursor: pointer could work on an ?"
(Just to clarify) Nope, that doesn't work, you'd have to apply the styling on the underlying path element(s)
…rxl881/invertOutlineColour
…rxl881/invertOutlineColour
Needed for - matrix-org/matrix-react-sdk#1611