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

Apply user's theme to the link tooltip #294

Closed
mickmister opened this issue Jun 5, 2020 · 7 comments · Fixed by #303
Closed

Apply user's theme to the link tooltip #294

mickmister opened this issue Jun 5, 2020 · 7 comments · Fixed by #303
Assignees
Labels
Difficulty/1:Easy Easy ticket Help Wanted Community help wanted Tech/ReactJS
Milestone

Comments

@mickmister
Copy link
Member

mickmister commented Jun 5, 2020

Currently the link tooltip is styled with a white background etc. It should instead match the user's theme to blend well with the center channel.

Screenshot from 2020-06-04 18-01-01

Issue created from a Mattermost message by @gabrieljackson.

@hanzei hanzei added Difficulty/1:Easy Easy ticket Help Wanted Community help wanted Tech/ReactJS Up For Grabs Ready for help from the community. Removed when someone volunteers labels Jun 5, 2020
@hanzei hanzei added this to the v1.1.0 milestone Jun 5, 2020
@hanzei
Copy link
Contributor

hanzei commented Jun 5, 2020

@fedealconada Would you be interested in working on this given that you introduced the feature?

@fedealconada
Copy link
Contributor

Sure @hanzei !

@mickmister mickmister removed the Up For Grabs Ready for help from the community. Removed when someone volunteers label Jun 5, 2020
@fedealconada
Copy link
Contributor

Hey @mickmister, I've linked the user's theme to the tooltip but there are some styles that are not passed in the theme's... I think that the things most affected are:

  1. Box shadow of the tooltip
  2. The little boxes specifying the repo branches (above See more link).

So, a couple of alternatives:
Option 1. We leave it like that (its not bad though).
Option 2. We add the Menu__content class to it, which will add some borders.
Option 3. We could add some additional classes to the theme but they could be a bit specific to the github tooltip

Option 1
image
On the following image, note the Repo <- Repo in which there's not enough contrast between the light blues
image
On the next two images, note the lack of box shadow (maybe not a big deal)
image
image

Option 2
On these images, I've added the Menu__content class. Note the rounded corners and the border color.
image
image
image
image

@hanzei
Copy link
Contributor

hanzei commented Jun 9, 2020

@asaadmahmood What do you think about these ideas ☝️

@asaadmahmood
Copy link
Contributor

@fedealconada We can keep the box shadow black.
However, the border color of the tooltip can be: center-channel-color-16.
Also, can you increase the z-index of the tooltips to be 1070?

@fedealconada
Copy link
Contributor

hey @asaadmahmood, I've just added that. It would look like this:
image
Thoughts?

@asaadmahmood
Copy link
Contributor

@fedealconada Looks great to me, one thing though, add a border-radius: 4px to it as well.

hanzei added a commit to fedealconada/mattermost-plugin-github that referenced this issue Jun 19, 2020
hanzei added a commit that referenced this issue Jun 25, 2020
Co-authored-by: Federico Martín Alconada Verzini <fedealconada@gmail.com>
Co-authored-by: Ben Schumacher <ben.schumacher@mattermost.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty/1:Easy Easy ticket Help Wanted Community help wanted Tech/ReactJS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants