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

unify use of tags$* across the package #445

Closed
10 of 14 tasks
chlebowa opened this issue Aug 16, 2023 · 5 comments · Fixed by #566
Closed
10 of 14 tasks

unify use of tags$* across the package #445

chlebowa opened this issue Aug 16, 2023 · 5 comments · Fixed by #566
Assignees
Labels

Comments

@chlebowa
Copy link
Contributor

chlebowa commented Aug 16, 2023

shiny functions that create HTML tags (re-exported from htmltools) can be used directly or as elements of the tags list:

div("some content")
tags$div("some content")

In teal.slice most div calls are direct but span calls usually use tags$.
Pick one and apply throughout the package.


similar changes done here as part of this PR

@kartikeyakirar
Copy link
Contributor

The majority of common HTML tags are directly accessible through re-exports from the htmltools package, including but not limited to a, br, code, div, em, h1 through h6, hr, HTML, htmlTemplate, img, includeCSS, includeHTML, includeMarkdown, includeScript, includeText, is.singleton, p, pre, singleton, span, strong, suppressDependencies, tag, tagAppendAttributes, tagAppendChild, tagAppendChildren, tagGetAttribute, tagHasAttribute, tagList, tags, tagSetChildren, validateCssUnit, and withTags. However, certain tags like td, tr, table, andlabelare exceptions and used in teal.sliceand are not re-exported in this manner; thus, they require tags$ prefix .

so for unifying I prefer to do prefix tags$ for all HTML tags to ensures a standardized codebase.

WDYT?

@chlebowa
Copy link
Contributor Author

chlebowa commented Mar 4, 2024

It's probably the way to go.

@averissimo
Copy link
Contributor

Good choice on the tags 👍 it will be easier to migrate if we move to shiny:: 😈

@chlebowa
Copy link
Contributor Author

chlebowa commented Mar 5, 2024

Good choice on the tags 👍 it will be easier to migrate if we move to shiny:: 😈

You can do that next month 😉

@m7pr
Copy link
Contributor

m7pr commented Mar 6, 2024

we can re-migrate every month : P

kartikeyakirar added a commit to insightsengineering/teal.transform that referenced this issue Mar 7, 2024
kartikeyakirar added a commit to insightsengineering/teal that referenced this issue Mar 8, 2024
part of insightsengineering/teal.slice#445
for teal package

---------

Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>
kartikeyakirar added a commit to insightsengineering/teal.widgets that referenced this issue Mar 14, 2024
part of insightsengineering/teal.slice#445
for teal.widget package

---------

Co-authored-by: André Veríssimo <[email protected]>
kartikeyakirar added a commit to insightsengineering/teal.modules.hermes that referenced this issue Mar 15, 2024
part of insightsengineering/teal.slice#445
for teal.modules.hermes package

Co-authored-by: André Veríssimo <[email protected]>
kartikeyakirar added a commit to insightsengineering/teal.goshawk that referenced this issue Mar 15, 2024
part of insightsengineering/teal.slice#445
for teal.goshawk package

---------

Co-authored-by: André Veríssimo <[email protected]>
kartikeyakirar added a commit to insightsengineering/teal.osprey that referenced this issue Mar 15, 2024
part of insightsengineering/teal.slice#445
for teal.osprey package

---------

Co-authored-by: André Veríssimo <[email protected]>
kartikeyakirar added a commit to insightsengineering/teal.modules.general that referenced this issue Mar 15, 2024
part of insightsengineering/teal.slice#445
for teal.modules.general package

---------

Co-authored-by: André Veríssimo <[email protected]>
Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>
kartikeyakirar added a commit that referenced this issue Mar 18, 2024
fixes #445

---------

Co-authored-by: André Veríssimo <[email protected]>
kartikeyakirar added a commit to insightsengineering/teal.modules.clinical that referenced this issue Mar 20, 2024
part of insightsengineering/teal.slice#445
for teal.modules.clinical package

---------

Co-authored-by: André Veríssimo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants