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 external link not hidden #48

Merged
merged 9 commits into from
Sep 12, 2024
Merged

Fix external link not hidden #48

merged 9 commits into from
Sep 12, 2024

Conversation

cjrace
Copy link
Contributor

@cjrace cjrace commented Sep 11, 2024

Brief overview of changes

After releasing the last version with external_link() we found a couple of bugs with the add_warning = FALSE version of the link.

Why are these changes being made?

Hidden text was not being hidden. Excess whitespace was being added. We had missed this in the tests.

Detailed description of changes

  • Attach CSS to the function to hide the hidden text.
  • Crudely paste and reparse the HTML to work around the whitespace issues with htmltools::tags.
  • Updated main package tests, and added some examples and tests to the test_dashboard.
  • Added some extra info into the documentation as suggested by @jen-machin.

Additional information for reviewers

zzz.R is apparently a specific standard file name for that kind of on package load function. shinyGovstyle uses and copilot independently also said this was the case (which was enough for me to just do it, feel free to verify this further!).

Issue ticket number/s and link

#47

@cjrace cjrace linked an issue Sep 11, 2024 that may be closed by this pull request
@cjrace cjrace changed the title Fix external link nothidden Fix external link not hidden Sep 11, 2024
…ng the cookies name match so snapshots match up
@cjrace cjrace marked this pull request as ready for review September 11, 2024 15:21
@cjrace
Copy link
Contributor Author

cjrace commented Sep 11, 2024

Copy link
Contributor

@jen-machin jen-machin left a comment

Choose a reason for hiding this comment

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

I have not spent a huge amount of time looking at every line of code but I have tested that everything works as it should 😄 thanks for the documentation updates!

@cjrace
Copy link
Contributor Author

cjrace commented Sep 12, 2024

Done a quick double check on the zzz.R thing and that is definitely a standard - https://r-pkgs.org/Code.html#when-you-do-need-side-effects

If you use .onLoad(), consider using .onUnload() to clean up any side effects. By convention, .onLoad() and friends are usually saved in a file called R/zzz.R. (Note that .First.lib() and .Last.lib() are old versions of .onLoad() and .onUnload() and should no longer be used.)

@cjrace cjrace merged commit bb02e44 into main Sep 12, 2024
10 checks passed
@cjrace cjrace deleted the fix-external-link-nothidden branch September 12, 2024 10:57
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.

external_link() hidden text is not hidden
2 participants