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

Addons: form for tooltips #521

Merged
merged 9 commits into from
Dec 5, 2024
Merged

Addons: form for tooltips #521

merged 9 commits into from
Dec 5, 2024

Conversation

humitos
Copy link
Member

@humitos humitos commented Nov 6, 2024

@humitos humitos requested a review from a team as a code owner November 6, 2024 09:52
@humitos humitos requested a review from agjohnson November 6, 2024 09:52
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

The technical changes here look fine, but I raised a point about the naming here. I would not call this element a "tooltip", this is a specific term for this element:

image

Tooltips are a text only element, usually used to describe a field/button/element, and these are normally not rich content elements. I'd call what we inject a popover or popup.

Noted in chat, but this is a clear review of the terms here:

https://medium.com/design-bootcamp/popups-dialogs-tooltips-and-popovers-ux-patterns-2-939da7a1ddcd

@humitos humitos requested a review from agjohnson November 12, 2024 12:34
@humitos
Copy link
Member Author

humitos commented Nov 12, 2024

I renamed "Tooltips" by "Link Previews"

@humitos
Copy link
Member Author

humitos commented Nov 12, 2024

We decided to not merge this PR yet so we can start testing it ourselves with our own projects first.

@humitos
Copy link
Member Author

humitos commented Nov 26, 2024

This PR needs to be adapted now that we have addons.options_root_selector.

@humitos humitos self-assigned this Nov 26, 2024
@humitos
Copy link
Member Author

humitos commented Nov 27, 2024

I updated this PR to show the "CSS main content selector" as a global option.

With the name changed to "Link previews" a new row is generated for the tabs and it doesn't render correctly. What's the solution here?

Screenshot_2024-11-27_17-57-10

@humitos humitos requested a review from agjohnson December 3, 2024 07:51
@humitos
Copy link
Member Author

humitos commented Dec 4, 2024

With the name changed to "Link previews" a new row is generated for the tabs and it doesn't render correctly. What's the solution here?

I think you missed my question from here, @agjohnson

@ericholscher
Copy link
Member

Sounds like there's a wrapped class we need here

@agjohnson
Copy link
Contributor

There is a wrapping class that the menu can use:

https://fomantic-ui.com/collections/menu.html#wrapping

However, that also looks pretty similar to what we have. The centered wrapping looks a little better though:

https://fomantic-ui.com/collections/menu.html#centered-fluid-wrapping

@humitos
Copy link
Member Author

humitos commented Dec 5, 2024

It still doesn't render perfectly to me, but I'll move forward with this.

Screenshot_2024-12-05_11-59-13

@humitos humitos merged commit 97706f2 into main Dec 5, 2024
4 checks passed
@humitos humitos deleted the humitos/addons-tooltips-form branch December 5, 2024 10:59
@ericholscher
Copy link
Member

Do we have docs for this? I'm assuming this is "shipping" this feature?

@humitos
Copy link
Member Author

humitos commented Dec 5, 2024

Good point. We don't have specific documentation for this yet. We only have the description of the backend API and a link to hoverxref in https://docs.readthedocs.io/en/stable/guides/embedding-content.html

I will try to quickly update that page to remove the recommendation for hoverxref and link to a new addons page for links preview with small content for now.

humitos added a commit to readthedocs/readthedocs.org that referenced this pull request Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Tooltips: sphinx-hoverxref popup implementation
3 participants