Skip to content

Make hover popover prettier #1183

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

Merged
merged 1 commit into from
Apr 23, 2024
Merged

Make hover popover prettier #1183

merged 1 commit into from
Apr 23, 2024

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Apr 18, 2024

Essentially two changes:

  1. Remove the symbol name from the hover. This name is already included in the documentation
  2. Make the Alternative Result heading smaller.

Changes it from looking like this
Screenshot 2024-04-17 at 21 41 27
to
Screenshot 2024-04-17 at 21 40 42

rdar://126488098

@ahoppen ahoppen requested a review from bnbarham April 18, 2024 04:43
@ahoppen ahoppen requested a review from benlangmuir as a code owner April 18, 2024 04:43
@bnbarham
Copy link
Contributor

Any thoughts on removing the horizontal rule after the declaration name and instead moving it prior to Alternative result? Needs something else too, since even with that I think I'd still start reading the bottom doc as the doc for the first result. Just... not sure what.

@ahoppen
Copy link
Member Author

ahoppen commented Apr 18, 2024

I did experiment with horizontal rulers to separate the results quite a bit and was never satisfied. FWIW, here’s how it looks like with a horizontal ruler before Alternative Result

Screenshot 2024-04-17 at 22 00 27

And here without any rulers

Screenshot 2024-04-17 at 22 01 22

@ahoppen
Copy link
Member Author

ahoppen commented Apr 18, 2024

And here’s a version that uses ###   to add some spacing between the results (best way I have found to add spacing in VS Code so far).

Screenshot 2024-04-17 at 22 06 48

@ahoppen
Copy link
Member Author

ahoppen commented Apr 20, 2024

Next try. Heading Multiple Results if there are multiple results and horizontal rulers between the results.

Screenshot 2024-04-19 at 22 37 48

@ahoppen
Copy link
Member Author

ahoppen commented Apr 20, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Apr 20, 2024

@swift-ci Please test Windows

@bnbarham
Copy link
Contributor

Nice, I think that's better. How about H3 still and with a horizontal line after the heading?

@ahoppen
Copy link
Member Author

ahoppen commented Apr 22, 2024

I prefer H2 and no line after the heading. Here’s how H3 and line after the heading looks like.

Screenshot 2024-04-22 at 10 50 35

@ahoppen
Copy link
Member Author

ahoppen commented Apr 22, 2024

@swift-ci Please test

@bnbarham
Copy link
Contributor

@swift-ci please test Windows platform

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

I still prefer H3 + line, but either is better than what's there now IMO. So happy to merge either way.

@ahoppen ahoppen merged commit 17c0a44 into swiftlang:main Apr 23, 2024
3 checks passed
@ahoppen ahoppen deleted the pretty-hover branch April 23, 2024 06:00
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.

2 participants