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

fix: Make popup on the Search input field less intrusive enhancement #1685

Merged
merged 3 commits into from
Aug 16, 2023

Conversation

anikdhabal
Copy link
Contributor

@anikdhabal anikdhabal commented Aug 15, 2023

Which problem is this PR solving?

#1651
Screenshot 2023-08-16 230437

Checklist

@yurishkuro
Copy link
Member

I like it. But can you reduce the white space around it, so that the button looks the same as the others on the right?

overlayStyle={{ maxWidth: '600px' }} // This is a large tooltip and the default is too narrow.
title={renderTooltip()}>
<Button htmlType="button"><IoHelp className="SearchForm--hintTrigger" /></Button>
</Tooltip>
{navigable && (
<>
<Button
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be nice to have a mouseover tooltip on this button, it's not very intuitive what it's supposed to do. The up/down arrows are more obvious, as well as [x] button.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-08-16 140219
@yurishkuro fixed.

Signed-off-by: Anik Dhabal Babu <adhabal2002@gmail.com>
Signed-off-by: Anik Dhabal Babu <adhabal2002@gmail.com>
@yurishkuro
Copy link
Member

you ticked the box, but are you sure you ran yarn lint / yarn test?

Signed-off-by: Anik Dhabal Babu <adhabal2002@gmail.com>
@anikdhabal
Copy link
Contributor Author

you ticked the box, but are you sure you ran yarn lint / yarn test?

sorry, silly mistake. I've fixed the lint and unit test.

@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (425fe08) 96.01% compared to head (bd4171e) 96.01%.
Report is 30 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1685   +/-   ##
=======================================
  Coverage   96.01%   96.01%           
=======================================
  Files         242      241    -1     
  Lines        7559     7560    +1     
  Branches     1984     1985    +1     
=======================================
+ Hits         7258     7259    +1     
  Misses        301      301           
Files Changed Coverage Δ
...s/TracePage/TracePageHeader/TracePageSearchBar.tsx 100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yurishkuro yurishkuro merged commit a5c02b5 into jaegertracing:main Aug 16, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants