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.

Sign up for GitHub

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

Jump to bottom

Add tooltip() API #662

Merged
merged 33 commits into from Jul 14, 2023
Merged

Add tooltip() API #662

merged 33 commits into from Jul 14, 2023

Conversation

cpsievert
Copy link
Collaborator

@cpsievert cpsievert commented Jul 6, 2023 •
edited

Adds tooltip(), update_tooltip(), and toggle_tooltip()

@cpsievert cpsievert marked this pull request as draft July 6, 2023 20:59
@cpsievert cpsievert requested a review from gadenbuie July 6, 2023 21:00
gadenbuie
gadenbuie reviewed Jul 10, 2023
View reviewed changes
R/tooltip.R Outdated Show resolved Hide resolved
R/tooltip.R Outdated Show resolved Hide resolved
R/tooltip.R Outdated Show resolved Hide resolved
cpsievert and others added 12 commits July 11, 2023 23:08
@cpsievert
devtools::document() (GitHub Actions)
a6a901d
@cpsievert
yarn build (GitHub Actions)
a8d442d
@cpsievert
Resave distributed files (GitHub Action)
e6ea4fe
@cpsievert
Make sure value_box()'s dependency doesn't create an empty card-body …
9db9d14 
…(i.e., card item) (#668)
@cpsievert @gadenbuie
Add input_switch() (#483)
149e9a6 
Co-authored-by: Garrick Aden-Buie <garrick@adenbuie.com>
@gadenbuie @cpsievert
fix(bs_theme_preview): Don't include dashboard tab for BS < 5 (#670)
44b0ed3
@gadenbuie @cpsievert
feat(bs_theme_preview): Add more DT features and more tables (#671)
5111433
@gadenbuie @cpsievert
feat(bs_theme_preview): Add more DT features and more tables (#671)
13e69e4
@gadenbuie @cpsievert
feat(card): Use an attribute to signal card is in full screen (#669)
17cddeb
@gadenbuie @cpsievert
Constrain width of showcase icons in value boxes (#653)
9683c4f
@cpsievert
Better tooltip_toggle() and tooltip_update() behavior; small improvem…
b254430 
…ents
@cpsievert
Merge branch 'main' into tooltips
2d587ec
@cpsievert cpsievert marked this pull request as ready for review July 12, 2023 23:50
@cpsievert
Copy link
Collaborator Author

@gadenbuie this is ready for another (closer) look

gadenbuie
gadenbuie reviewed Jul 13, 2023
View reviewed changes
Copy link
Member

@gadenbuie gadenbuie left a comment

Choose a reason for hiding this comment

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

Nice! Here are comments and thoughts from the R side, I'll read through the JS side later today.

NAMESPACE Outdated Show resolved Hide resolved
R/card.R Show resolved Hide resolved
R/tooltip.R Outdated Show resolved Hide resolved
R/tooltip.R Outdated Show resolved Hide resolved
R/tooltip.R Outdated Show resolved Hide resolved
gadenbuie
gadenbuie reviewed Jul 13, 2023
View reviewed changes
srcts/src/components/tooltip.ts Outdated Show resolved Hide resolved
srcts/src/components/tooltip.ts Outdated Show resolved Hide resolved
srcts/src/components/tooltip.ts Outdated Show resolved Hide resolved
R/tooltip.R Outdated Show resolved Hide resolved
R/tooltip.R Outdated

#' @describeIn tooltip Update the contents of a tooltip.
#' @export
tooltip_update <- function(id, ..., session = get_current_session()) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we support updating other aspects of the tooltip, like placement or options?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Potentially, maybe in the future?

srcts/src/components/tooltip.ts Outdated Show resolved Hide resolved
gadenbuie
gadenbuie approved these changes Jul 14, 2023
View reviewed changes
Copy link
Member

@gadenbuie gadenbuie left a comment

Choose a reason for hiding this comment

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

Awesome! :shipit:

@cpsievert cpsievert merged commit 8f255e5 into main Jul 14, 2023
1 check passed
@cpsievert cpsievert deleted the tooltips branch July 14, 2023 19:43
@cpsievert cpsievert mentioned this pull request Jul 14, 2023
Merged
This was referenced Jul 19, 2023
Closed
Merged
Merged
@cpsievert cpsievert mentioned this pull request Jul 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gadenbuie gadenbuie gadenbuie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@cpsievert @gadenbuie