feat: add tooltip trigger #694

Vlad116 · 2023-06-07T13:21:33Z

Brief Information

This pull request is in the type of (more info about types):

Related issues (all available keywords):

close [Improvement] How to trigger Tooltips in settings #692

Details

Added a TooltipTrigger component with a question mark icon (accepts color and size as a string).

- Added a tooltip visibility toggle at the top of the page

- Added a small margin at the bottom for the main content (it could overlap with the operating system navigation menu)

Checklist

I have read the CONTRIBUTING doc
I have signed the CLA

Others

Demo:
https://github.com/hypertrons/hypertrons-crx/assets/32329117/788d1878-9dcb-4a8c-99fd-e4d76924c825

…ons#692), ♻️

CLAassistant · 2023-06-07T13:21:40Z

All committers have signed the CLA.

wxharry · 2023-06-07T15:06:00Z

Hi, Vladislav. Thanks for your contribution. You've open a PR before we could fully discuss. Here are my feedback on this PR:

change the name of the PR. We encourage contributors to format their commit message in our project. Please refer to angular commit-message-format for more details
The tooltip should be considered as a component. Could you move the src/pages/Options/TooltipTrigger/index.tsx to the component folder so that the code could be well-organized.
Remove the tooltips enable/disable switch. Since you jump to pr, my opinion is to change how to trigger the tooltips only, we don't need that switch for now.
Move the icon to the left. This could be optional depending on the implementation. The final goal to this issue is to make this tooltips more intuitive. My expectation was like this. Let's see if you could make it and we could discuss about it later.
Organize your code. Working at an open source project is not just make it work, you could refer to the existing code to better organize your work for future maintenance

If you have any questions, feel free to comment below or contact us on slack. Again, thanks for your interests and contributions to the project. 😄

Vlad116 · 2023-06-07T15:36:38Z

Hi, Harry) Sure, i can move the icon like on your screnshot and edit tooltip area how on it, also remove switch.
I did not move TooltipTrigger to components, because often non-reusable components are made as subcomponents with index file.
Okay, I'll correct all comments. Could you clarify on the 5th point what exactly is wrong with the organization of the code if we abstract from the points indicated earlier?

Vlad116 · 2023-06-07T15:44:12Z

Is the current icon right for you? @wxharry

wxharry · 2023-06-07T16:51:24Z

Is the current icon right for you? @wxharry

I was expecting an information mark like in the previous screenshot if possible.

Hi, Harry) Sure, i can move the icon like on your screnshot and edit tooltip area how on it, also remove switch. I did not move TooltipTrigger to components, because often non-reusable components are made as subcomponents with index file.

I understand. I was expecting it to be resuable because we are planning some new features in this option page, so we might reuse the tooltips in the near future.

Could you clarify on the 5th point what exactly is wrong with the organization of the code if we abstract from the points indicated earlier?

Sure. For example, you could refer to src/pages/ContentScripts/features/developer-networks to extract the icon. I understand it might not be the best way to do this, but we need to keep it identical within the project. Thanks for understanding.

…pertrons#692), ♻️

Vlad116 · 2023-06-08T07:12:20Z

Hello!)
I tried to make a dark background with transparency like in your screenshot, but it turned out to be not so easy with the implementation of the tooltip in this library, you also can pass background/fontColor/iconColor to TooltipTrigger component. I removed the switch (and moved the TooltipTrigger to shared components), brought the tooltips to the following form:

Vlad116 · 2023-06-09T07:27:39Z

Are there any other changes needed to the current version or will we be able to close this PR soon? @wxharry

wxharry · 2023-06-09T12:45:47Z

Are there any other changes needed to the current version or will we be able to close this PR soon? @wxharry

Slow down. Thanks for your passion but there is no need to rush. I am running some tests and thinking about some improvements.

Vlad116 · 2023-06-09T13:08:15Z

Are there any other changes needed to the current version or will we be able to close this PR soon? @wxharry

Slow down. Thanks for your passion but there is no need to rush. I am running some tests and thinking about some improvements.

Of course, I'll just find out if any more updates are needed, I will check this thread once a day to find out

wxharry · 2023-06-12T13:50:21Z

It's fine if we cannot make it transparent. just make it to the default style as before will do.

Vlad116 · 2023-06-15T22:03:43Z

It's fine if we cannot make it transparent. just make it to the default style as before will do.

I'm update MR and fix default style for TooltipTrigger @wxharry

wxharry

Everything looks good to me, except that it feels unnatural to me change the cursor to a question mark when hovering over the icon. Could you remove it?

I understand this style given by the demo I previously provided. Sorry for letting you change the style again and again!

Vlad116 · 2023-06-27T06:22:29Z

@wxharry Okay, no problem. I can change to default cursor style, sorry for awaiting

…hypertrons-crx into feature/tooltip-trigger

wxharry

LGTM! Thanks!
/approve

* feat: add switchable TooltipTrigger to settings page, 💄 * feat: add isTooltipEnabled state and stackStyleOptions const (hypertrons#692), ♻️ * feat: change cursor css prop to 'help' (hypertrons#692), 💄 * feat: remove Toggle button and move TooltipTrigger to /components (hypertrons#692), ♻️ * feat: fix TooltipTrigger styles (hypertrons#692), ♻️ 💄 * feat: fix defaultStyle for tooltip (hypertrons#692) 💄 * feat: change cursor to 'default' on tooltip icon 💄

Vlad116 added 3 commits June 7, 2023 15:04

feat: add switchable TooltipTrigger to settings page, 💄

c8cbfdb

feat: add isTooltipEnabled state and stackStyleOptions const (hypertr…

16e957e

…ons#692), ♻️

feat: change cursor css prop to 'help' (hypertrons#692), 💄

3df9d11

menbotics bot added the kind/feature Category issues or prs related to feature request. label Jun 7, 2023

Vlad116 changed the title ~~Feature/tooltip trigger~~ feat: add tooltip trigger Jun 7, 2023

Vlad116 added 2 commits June 8, 2023 08:52

feat: remove Toggle button and move TooltipTrigger to /components (hy…

d3db459

…pertrons#692), ♻️

feat: fix TooltipTrigger styles (hypertrons#692), ♻️ 💄

dc8df59

Vlad116 force-pushed the feature/tooltip-trigger branch from 1c4f5cd to dc8df59 Compare June 8, 2023 07:06

Vlad116 and others added 2 commits June 16, 2023 00:57

feat: fix defaultStyle for tooltip (hypertrons#692) 💄

e7fba0d

Merge branch 'hypertrons:master' into feature/tooltip-trigger

c11e1ae

wxharry requested changes Jun 23, 2023

View reviewed changes

Vlad116 added 2 commits June 27, 2023 09:31

feat: change cursor to 'default' on tooltip icon 💄

fbed21e

Merge branch 'feature/tooltip-trigger' of https://github.com/Vlad116/…

5025847

…hypertrons-crx into feature/tooltip-trigger

Vlad116 requested a review from wxharry June 27, 2023 06:35

wxharry approved these changes Jun 28, 2023

View reviewed changes

menbotics bot added the pull/approved If a pull is approved, it will be automatically merged label Jun 28, 2023

menbotics bot merged commit a0764e3 into hypertrons:master Jun 28, 2023

menbotics bot mentioned this pull request Jul 3, 2023

[WeeklyReport] Weekly report for hypertrons-crx 6/26/2023 to 7/3/2023 #700

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add tooltip trigger #694

feat: add tooltip trigger #694

Vlad116 commented Jun 7, 2023 •

edited

Loading

CLAassistant commented Jun 7, 2023 •

edited

Loading

wxharry commented Jun 7, 2023

Vlad116 commented Jun 7, 2023

Vlad116 commented Jun 7, 2023

wxharry commented Jun 7, 2023

Vlad116 commented Jun 8, 2023 •

edited

Loading

Vlad116 commented Jun 9, 2023

wxharry commented Jun 9, 2023

Vlad116 commented Jun 9, 2023

wxharry commented Jun 12, 2023

Vlad116 commented Jun 15, 2023 •

edited

Loading

wxharry left a comment

Vlad116 commented Jun 27, 2023

wxharry left a comment •

edited

Loading

feat: add tooltip trigger #694

feat: add tooltip trigger #694

Conversation

Vlad116 commented Jun 7, 2023 • edited Loading

Brief Information

Details

Checklist

Others

CLAassistant commented Jun 7, 2023 • edited Loading

wxharry commented Jun 7, 2023

Vlad116 commented Jun 7, 2023

Vlad116 commented Jun 7, 2023

wxharry commented Jun 7, 2023

Vlad116 commented Jun 8, 2023 • edited Loading

Vlad116 commented Jun 9, 2023

wxharry commented Jun 9, 2023

Vlad116 commented Jun 9, 2023

wxharry commented Jun 12, 2023

Vlad116 commented Jun 15, 2023 • edited Loading

wxharry left a comment

Choose a reason for hiding this comment

Vlad116 commented Jun 27, 2023

wxharry left a comment • edited Loading

Choose a reason for hiding this comment

Vlad116 commented Jun 7, 2023 •

edited

Loading

CLAassistant commented Jun 7, 2023 •

edited

Loading

Vlad116 commented Jun 8, 2023 •

edited

Loading

Vlad116 commented Jun 15, 2023 •

edited

Loading

wxharry left a comment •

edited

Loading