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: API response for tooltips #11746

Merged
merged 8 commits into from
Nov 12, 2024
Merged

Addons: API response for tooltips #11746

merged 8 commits into from
Nov 12, 2024

Conversation

humitos
Copy link
Member

@humitos humitos commented Nov 5, 2024

Initial addons.tooltips object with basic data for testing purposes.

We will also want to add here:

  • ext-theme addons configurations for tooltips
  • model fields for tooltips
  • disable it by default for now
  • tests for the new API response

Required by readthedocs/addons#422
Closes readthedocs/addons#239

Initial `addons.tooltips` object with basic data for testing purposes.
@humitos humitos requested a review from a team as a code owner November 5, 2024 19:25
@humitos humitos requested a review from stsewd November 5, 2024 19:25
@humitos humitos marked this pull request as draft November 5, 2024 19:25
@humitos humitos marked this pull request as ready for review November 6, 2024 09:55
Copy link
Member

@stsewd stsewd left a comment

Choose a reason for hiding this comment

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

Tests are failing, this will probably require to update tests on .com as well.

@humitos humitos requested a review from stsewd November 7, 2024 11:10
Comment on lines +229 to +240
linkpreviews_doctool_name = models.CharField(
choices=LINKPREVIEWS_DOCTOOL_NAME_CHOICES,
null=True,
blank=True,
max_length=128,
)
linkpreviews_doctool_version = models.CharField(
null=True,
blank=True,
max_length=128,
)

Copy link
Member

@ericholscher ericholscher Nov 12, 2024

Choose a reason for hiding this comment

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

Are we planning to auto-fill this via the build config or something? Feels odd that users would have to define this in multiple places. In particular a version number feels super weird to have to keep updated, and definitely won't be modified by the user in any ongoing way.

@ericholscher
Copy link
Member

Will this start showing for everyone, or are we not shipping the user facing UI here yet?

@humitos
Copy link
Member Author

humitos commented Nov 12, 2024

Link previews are gonna be disabled by default. At least for now. We can change that in the future if we consider.

The UI is at readthedocs/ext-theme#521, so we can decide merge it or not.

@ericholscher ericholscher merged commit 8cc2b44 into main Nov 12, 2024
8 checks passed
@ericholscher ericholscher deleted the humitos/addons-tooltips branch November 12, 2024 15:45
@ericholscher
Copy link
Member

Will plan to merge this, and test it via the admin with no user-facing UI to start.

humitos added a commit to readthedocs/ext-theme 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
None yet
Development

Successfully merging this pull request may close these issues.

Tooltips: sphinx-hoverxref popup implementation
3 participants