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

feat: added a verbatim_popup module #59

Merged
merged 4 commits into from
Jun 20, 2022
Merged

Conversation

kpagacz
Copy link
Contributor

@kpagacz kpagacz commented Jun 14, 2022

Closes #30

Test with the example replacing the contents of the modal. Also, have a look at the tests for examples of what should be allowed and what should not be allowed into the modal.

The idea is that behaviour is the same as the previous get_rcode module but without the get_eval_details part.

This just shows the content formatted as verbatim. Be aware that it will not format properly without shinys bundled CSS, so it will not look as intended when the UI function is called outside of a shiny object (like a fluid page in the example).

I replaced the bloated clipboard.js with our own, custom js. Should be short enough to maintain.

I would be glad if someone with MacOS tested the copying function because I have read about some edge cases with the Navigator API and MacOS. I have put some fallback code inspired by an so post, but I would sleep better if it was tested.

@kpagacz kpagacz added the core label Jun 14, 2022
Comment on lines +45 to +48
if (inherits(verbatim_content, "reactive")) {
shiny::validate(shiny::need(
checkmate::assert_multi_class(verbatim_content(), classes = c("expression", "character"))
))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially, I had this validation somewhere else, now I am not sure it needs to be here or inside the module function (I think inside the module :( )

@github-actions
Copy link
Contributor

github-actions bot commented Jun 14, 2022

Code Coverage Summary

Filename                   Stmts    Miss  Cover    Missing
-----------------------  -------  ------  -------  -------------------------
R/accordion.R                 45      45  0.00%    12-62
R/basic_table_args.R          23       0  100.00%
R/draggable_buckets.R         84      84  0.00%    44-141
R/get_dt_rows.R               13      13  0.00%    27-39
R/ggplot2_args.R              49       0  100.00%
R/include_css_js.R             7       7  0.00%    12-19
R/optionalInput.R            201     201  0.00%    78-458
R/panel_group.R               68      68  0.00%    15-127
R/plot_with_settings.R       377     371  1.59%    8-582
R/standard_layout.R           40       0  100.00%
R/table_with_settings.R      167     167  0.00%    11-236
R/verbatim_popup.R            76      48  36.84%   43-65, 80-81, 83, 104-128
R/white_small_well.R           5       5  0.00%    18-22
TOTAL                       1155    1009  12.64%

Results for commit: c5c20ba

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Jun 14, 2022

Unit Tests Summary

  1 files    5 suites   1s ⏱️
55 tests 55 ✔️ 0 💤 0
99 runs  99 ✔️ 0 💤 0

Results for commit edbb8de.

♻️ This comment has been updated with latest results.

Comment on lines +23 to +24
export(verbatim_popup_srv)
export(verbatim_popup_ui)
Copy link
Contributor

Choose a reason for hiding this comment

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

please modify _pkgdown accordingly

Copy link
Contributor

Choose a reason for hiding this comment

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

I am just curious why pkgdown workflow does not complain. That should be catched automatically. Will you be able to have a look at this and create a task for IDR if necessary? Many thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

? It is there, isn't it?
image

Copy link
Contributor

Choose a reason for hiding this comment

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

brain fart - sorry

Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

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

Wundabar. Please add lifecycle experimental badges and add pkgdown entries.

@kpagacz
Copy link
Contributor Author

kpagacz commented Jun 17, 2022

Badges added. Pkgdown unchanged, it already lists verbatim_output which is the name of the roxygen topic.

Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

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

🚀

@kpagacz kpagacz merged commit 6bcc9b1 into main Jun 20, 2022
@kpagacz kpagacz deleted the 30_show_rcode_module@main branch June 20, 2022 07:38
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 this pull request may close these issues.

Make a show_rcode module
4 participants