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

Add 309: Test flexdashboard tab and page management with {bslib} #155

Merged
merged 12 commits into from
May 9, 2023

Conversation

gadenbuie
Copy link
Member

@gadenbuie gadenbuie commented Mar 20, 2023

Adds an app for testing tab and page management that rely on Bootstraps Tab plugin, as managed by bslib, over Bootstrap version 3, 4 and 5.

Fixes #154

@gadenbuie gadenbuie requested a review from schloerke March 20, 2023 16:22
}

expect_test_element_hidden <- function(app, test_id) {
expect_false(app$get_js(is_test_element_visible(!!test_id)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Since these tests are essentially purely client-side, it'd probably faster/better to use {shinyjster} instead of {shinytest2}, for example:

jster <- shinyjster::shinyjster_js(
"
var jst = jster(0);
jst.add(Jster.shiny.waitUntilStable);
jst.add(function() {
var toggle = $('.navbar-toggle:visible');
var nav = $('.navbar-collapse:visible');
Jster.assert.isEqual(toggle.length, 1, 'Failed to find collapsible menu, does the window need to be resized?');
Jster.assert.isEqual(nav.length, 0, 'The collapsible navbar should not be visible by default');
toggle.click();
});
// wait for nav to open
jst.add(function(done) {
var wait = function() {
if ($('.navbar-collapse:visible').length > 0) {
done();
} else {
setTimeout(wait, 5);
}
}
wait();
});
jst.add(function() {
Jster.assert.isEqual(
$('.navbar-collapse:visible').length, 1,
'Clicking the navbar toggle should make the navbar appear.'
);
});
jst.test();
"
)

Copy link
Member Author

Choose a reason for hiding this comment

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

@schloerke Is shinyjster designed to facilitate running and testing R Markdown Shiny apps using runtime: shiny? (I couldn't find anything in the docs.)

It seems like the biggest advantage of using shinyjster would be to test in Chrome, Edge, and Firefox. From looking at the logs of previous runs, though, and unless I'm misunderstanding them, I would expect switching to shinyjster to increase the test time for these tests. Right now, they take ~7 seconds locally on my M1 for all three Bootstrap variants. With shinyjster we'd be adding two more browsers into the matrix.

Copy link
Contributor

Choose a reason for hiding this comment

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

{shinyjster} can run inside Rmd docs. Ex:

```{r}
## `{shinyjster}` note:
# From https://github.com/rstudio/shiny/issues/3780, we must delay the underlying initial
# call to `Shiny.setInputValue("jster_initialized", true)` due to changes in https://github.com/rstudio/shiny/pull/3666.
# Current stance is that https://github.com/rstudio/shiny/issues/3780 will not be resolved, so we must make a work around.
# This is done by delaying the initial call to `Shiny.setInputValue("jster_initialized", true)`
# by using a dynamic UI that is invalidated on the first draw, and then actually rendered on the second draw.
renderUI({
shinyjster::shinyjster_js(
"
var jst = jster();
jst.add(Jster.shiny.waitUntilStable);
jst.add(function(done) {
var wait = function() {
var txt = $('#status').get(0).textContent;
if (
typeof txt == 'string' &&
txt.length > 0 &&
(txt.match(new RegExp('Pass|Fail')) ?? '').length > 0
) {
done();
return;
}
setTimeout(wait, 100);
}
wait();
})
jst.add(function() {
Jster.assert.isEqual(
$('#status').text().trim(),
'Pass'
)
})
jst.test();
"
)
})
shinyjster::shinyjster_server(input, output)
```

Yes, it's reasonable that the time would be increased, but it is not by a large factor. I'd be comfortable using shinyjster.

@schloerke
Copy link
Contributor

@schloerke schloerke marked this pull request as draft May 2, 2023 18:35
@gadenbuie gadenbuie changed the title Test flexdashboard tab and page management with {bslib} Add 309: Test flexdashboard tab and page management with {bslib} May 3, 2023
@cpsievert
Copy link
Contributor

cpsievert commented May 8, 2023

This currently fails, as expected, for Bootstrap 5 with the CRAN version of flexdashboard (0.6.1) and latest bslib.

Does this really matter though since {shinycoreci} tests against the github version of {flexdashboard} (which has a fix for this)? All the tests seem to be passing so this seems ok to merge as is?

@gadenbuie gadenbuie marked this pull request as ready for review May 9, 2023 10:38
@gadenbuie
Copy link
Member Author

Does this really matter though since {shinycoreci} tests against the github version of {flexdashboard} (which has a fix for this)? All the tests seem to be passing so this seems ok to merge as is?

Yeah I think this is all set to merge and fine to merge ahead of rstudio/bslib#501.

@gadenbuie gadenbuie merged commit 57e8734 into main May 9, 2023
@gadenbuie gadenbuie deleted the 309-flexdashboard-tabs-navs branch May 9, 2023 17:46
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.

Add a test for flexdashboard tabs and navigation pages
3 participants