fix: Tooltips point in wrong direction#11935#12200
fix: Tooltips point in wrong direction#11935#12200JessieTeng89 wants to merge 5 commits intoargoproj:masterfrom
Conversation
Signed-off-by: Teng, Jessie <yilin.teng@fmr.com>
Signed-off-by: Teng <yilin.teng@fmr.com>
JessieTeng89
force-pushed
the
fidelity-20230119-153729
branch
from
bbdcd4d to
1b5385a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12200 +/- ##
=======================================
Coverage 47.44% 47.44%
=======================================
Files 246 246
Lines 41864 41864
=======================================
Hits 19862 19862
Misses 20004 20004
Partials 1998 1998 ☔ View full report in Codecov by Sentry. |
|
|
||
| const tooltipProps = { | ||
| onShow: (instance: any) => { | ||
| instance.set({ |
There was a problem hiding this comment.
Is it really not possible to do this in pure css?
There was a problem hiding this comment.
@jsoref I tried to investigate this again, since this tooltip is using a third-party component, all the styles are controlled by itself, but it can be covered when it is initialized just like the code change in this PR.
But also if you don't want to change any code in javascript, we have another option, we need to add a class name on the container level of the tooltip, and then add media query rules for different screen sizes in css.
Please let me know which way you preferred.
There was a problem hiding this comment.
In the long term, at least an issue to the 3rd party so that we could eventually fix this with css would be great.
I'm not able to approve, so I can't say whether the project is ok with your current proposal.
If this were my project and you wrote the update ticket+PR, I'd accept this change in the short term.
There was a problem hiding this comment.
@jsoref Thanks for your comments, either way is good for me, let me ask the owners' thoughts.
Hi @alexec @rbreeze
Could you please help on this PR? Please find more details below:
The issue the sidebar tooltip width is too large for responsive in small screens(less than 590px), I created the PR to change the tooltip width dynamically when it is initialized, but this needs change the code of javascript.
Another solution is we can add a class name on the container level of the tooltip, and then add media query rules for different screen sizes in css, in this way, we only need to change HTML and CSS.
Could you please leave your comment which way you preferred so this can be merged?
There was a problem hiding this comment.
I agree with @jsoref that this should be taken care by css. That is the media query way, there are many components which need to be responsive in the UI and it would be best to have a consistent approach for all of them.
There was a problem hiding this comment.
@saumeya Thanks for your suggestion, I will create another PR with media query way to fix this issue.
|
Thanks a lot for the contribution @JessieTeng89 , could you also attach the screenshot of the change in description? Thanks, will review it. |
Hi, @saumeya @jsoref , After changed, you can see the result after changed in small window with 500px |
|
Looks good to me |
|
Close this PR as another related PR was merged |
3 participants
fix: Tooltips point in wrong direction
Fixes #11935