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

Make sure that the examples take up the full page width #12

Merged
merged 7 commits into from
Nov 6, 2024

Conversation

vedhav
Copy link
Contributor

@vedhav vedhav commented Nov 4, 2024

Closes #5
Here is a screenshot of the reference page with multiple examples.

screencapture-file-Users-vedha-insightsengineering-teal-modules-general-docs-reference-tm-g-response-html-2024-11-04-20_19_17

@vedhav vedhav requested a review from pawelru as a code owner November 4, 2024 14:51
Copy link
Contributor

github-actions bot commented Nov 4, 2024

✅ All contributors have signed the CLA
Posted by the CLA Assistant Lite bot.

@vedhav
Copy link
Contributor Author

vedhav commented Nov 4, 2024

I have read the CLA Document and I hereby sign the CLA

@vedhav vedhav enabled auto-merge (squash) November 4, 2024 14:56
@pawelru
Copy link
Contributor

pawelru commented Nov 5, 2024

Is this what we expect?

  • pkgdown
    image
  • manual in webbrowser
    image

@vedhav
Copy link
Contributor Author

vedhav commented Nov 5, 2024

I see you have a wider monitor screen. I think the optimal thing here would be to set a maximum width that looks good. Which is generally a 16:8 aspect ratio and given that we have an 800px height the max width can be 1400px. Let me make this change.

Copy link
Contributor

github-actions bot commented Nov 5, 2024

Unit Tests Summary

 1 files   2 suites   0s ⏱️
15 tests 15 ✅ 0 💤 0 ❌
54 runs  54 ✅ 0 💤 0 ❌

Results for commit 579f7b9.

♻️ This comment has been updated with latest results.

@vedhav
Copy link
Contributor Author

vedhav commented Nov 5, 2024

How about now? It is centered with a max-width. But it will still have the window's width as the normal width to feel responsive when width is reduced.
screencapture-file-D-insightsengineering-teal-modules-general-docs-reference-tm-data-table-html-2024-11-05-22_04_35

@pawelru
Copy link
Contributor

pawelru commented Nov 5, 2024

This one is great. Thank you!

image

@vedhav
Copy link
Contributor Author

vedhav commented Nov 5, 2024

haha we both posted at the exact same time!

@pawelru
Copy link
Contributor

pawelru commented Nov 5, 2024

I am happy with the outcome. Please just have a look at the R CMD CHECK results - tidy HTML is failing. This is because tidy will be used on the package that uses roxy.shinylive and create R CMD NOTE. So we need to care about this in here.

Copy link
Contributor

github-actions bot commented Nov 6, 2024

badge

Code Coverage Summary

Filename                     Stmts    Miss  Cover    Missing
-------------------------  -------  ------  -------  ---------
R/parse_url.R                   12       0  100.00%
R/tag_examplesShinylive.R       71       0  100.00%
TOTAL                           83       0  100.00%

Diff against main

Filename                     Stmts    Miss  Cover
-------------------------  -------  ------  --------
R/tag_examplesShinylive.R       -1       0  +100.00%
TOTAL                           -1       0  +100.00%

Results for commit: 579f7b9

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@vedhav
Copy link
Contributor Author

vedhav commented Nov 6, 2024

@pawelru I remember we discussed moving the shinylive examples below the shiny examples code. I tried to make this happen but couldn’t figure it out I'd differ on your roxygen black magic skills.

@pawelru
Copy link
Contributor

pawelru commented Nov 6, 2024

copy&paste from slack if one will be looking into this

The order is hardcoded in pkgdown here. roxy.shinylive creates a section and every section is before examples. pkgdown API is not very open to provide your own template. And even if it is - it would go beyond the aimed complexity of roxy.shinylive . I'm afraid that we need to live with this...

Yes, we can provide a custom template as a file in the repo (reference). This way we can render #sections then #examples and then our custom #xyz .
The problem then is how to make our stuff to be stored as a custom xyz . This goes probably beyond pkgdown into roxygen2. Currently, roxygen2 can only be "extended" with a custom section (manual).
This looks to me that the template only allows for custom html tweaks (incl. css and js) but not necessarily adding a new element (on the level of existing section).
Regardless - the whole idea was to have a small extension that is easily implementable. This whole thing would require additional files and that's a complexity that I would like to avoid.

Here is the piece of code that creates an object which is resolved against that template I shared earlier. The returned object is created from scratch and it has a fixed structure - it does not allow to create a new xyz element that could be potentially resolved using #xyz in the template.
This function also hardcodes the order of the sections - here - in particular: not the template. roxygen2 only allows (or instructs) how to write into the section element of this. As we can see it's going to be rendered between seealso and author.

Copy link
Contributor

@pawelru pawelru left a comment

Choose a reason for hiding this comment

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

Thank you!

@vedhav vedhav merged commit 24dadaa into main Nov 6, 2024
26 checks passed
@vedhav vedhav deleted the 5-full-width-iframe@main branch November 6, 2024 08:58
@github-actions github-actions bot locked and limited conversation to collaborators Nov 6, 2024
@pawelru
Copy link
Contributor

pawelru commented Nov 6, 2024

Now we are expecting that PRs to both tmg and tmc will re-generate the docs using this new version of roxy.shinylive and we are expecting some changes in the .Rd files.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Question]: How to properly add examples so shinylive works properly?
2 participants