Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TintableSvgButton styling #5605

Merged
merged 5 commits into from Nov 15, 2017

Conversation

@rxl881
Copy link
Contributor

rxl881 commented Nov 15, 2017

@@ -332,3 +332,30 @@ textarea {
border: 1px solid gray;
background-color: darkred;
}

.mx_TintableSvgButton {

This comment has been minimized.

Copy link
@ara4n

ara4n Nov 15, 2017

Member

I'm very surprised that the TintableSvgButton CSS was wrong, and i'm failing to see why this is needed? What's the story? :)

This comment has been minimized.

Copy link
@ara4n

ara4n Nov 15, 2017

Member

oh, having read the react-sdk stuff this makes a bit more sense. thinks

This comment has been minimized.

Copy link
@ara4n

ara4n Nov 15, 2017

Member

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/>?

This comment has been minimized.

Copy link
@rxl881

rxl881 Nov 15, 2017

Author Contributor

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?

This comment has been minimized.

Copy link
@ara4n

ara4n Nov 15, 2017

Member

ok, lgtm. still dubious about having two spans introduced though (as per other PR)

This comment has been minimized.

Copy link
@rxl881

rxl881 Nov 15, 2017

Author Contributor

"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 rxl881 assigned ara4n and unassigned ara4n Nov 15, 2017
@rxl881 rxl881 merged commit 53a793b into develop Nov 15, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@rxl881 rxl881 deleted the rxl881/invertOutlineColour branch Nov 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.