Bugfix/32 by Eduardodudu · Pull Request #42 · Appsilon/shiny.emptystate

Eduardodudu · 2023-08-01T15:27:25Z

Have you read the Contributing Guidelines?

Issue #32

Changes description

Created a new js function shiny_emptystate_updatePosition that updates the position of the empty container when the selected tag id is triggered

Example:

shiny::shinyApp(
    ui = bslib::page(
      theme = bslib::bs_theme(version = 5),
      use_empty_state(),
      shiny::actionButton(
        "toggle_pannel",
        "Toggle panel",
        class = "btn btn-primary",
        onClick = "$('#container1').toggle(anim = 'slide', function(){shiny_emptystate_updatePosition('container2')});"
      ),
      shiny::div(
        style = "width: 300px",
        class = "d-flex flex-column gap-5",
        shiny::div(
          id = "container1",
          shiny::div(
            shiny::h1("Card 1"),
            "Card content"
          )
        ),
        shiny::div(
          id = "container2",
          shiny::div(
            shiny::h1("Card 2"),
            "Card content"
          )
        )
      )
    ),
    server = function(input, output) {
      empty_state_content <- shiny::div(
        "This is example empty state content"
      )
      empty_state_manager <- EmptyStateManager$new(
        id = "container2",
        html_content = empty_state_content
      )
      empty_state_manager$show()
    }
  )

Clearly and concisely describe what's in this pull request. Include screenshots, if necessary.

Eduardodudu · 2023-08-01T15:31:48Z

Approaches I tried before this bugfix

These were two approaches I tried by using an observer event to trigger an update of the emptyStateContainer to follow along the animation slider. These approaches didn't work and I ended up using the after animation slide approach.

`IntersectionObserver` with `getBoundingClientRect`

//create the intersection observer
const intersectionObserver = new IntersectionObserver((entries) => {

  entries.forEach(entry => {
    const emptyStateContainer = entry.target.querySelector(".empty-state-container");
    const rect = entry.target.getBoundingClientRect();
    const isVisible = entry.isIntersecting;

    if (isVisible) {
      emptyStateContainer.style.height = rect.offsetHeight + "px";
      emptyStateContainer.style.width = rect.offsetWidth + "px";
      emptyStateContainer.style.left = rect.offsetLeft + "px";
      emptyStateContainer.style.top = rect.offsetTop + "px";
      console.log(emptyStateContainer.style);
      console.log(rect.style);
    }
  });
}, {
  root: null,
  rootMargin: '0px',
  threshold: 1.0 // Fully visible
});


//use it on the show and hide methods
function showEmptyState(message) {
 // ....
  intersectionObserver.observe(elementToReplace);
}

function hideEmptyState(message) {
	// ...
  intersectionObserver.unobserve(elementToReplace);
}

Using MutationObserver

// ...

function showEmptyState(message) {
	//...
  intersectionObserver.observe(elementToReplace);
  observeDOMChanges(elementToReplace);
}

function repositionEmptyState() {
  const emptyStateContainers = document.querySelectorAll(".empty-state-container");

  emptyStateContainers.forEach(emptyStateContainer => {
    const parentElement = emptyStateContainer.parentElement;
    emptyStateContainer.style.height = parentElement.offsetHeight + "px";
    emptyStateContainer.style.width = parentElement.offsetWidth + "px";
    emptyStateContainer.style.left = parentElement.offsetLeft + "px";
    emptyStateContainer.style.top = parentElement.offsetTop + "px";
  });
}

function observeDOMChanges(elementToReplace) {
  const targetNode = elementToReplace;

  const mutationObserver = new MutationObserver(mutationsList => {
    repositionEmptyState();
  });

  const config = { attributes: true, childList: true, subtree: true };

  mutationObserver.observe(targetNode, config);
}

jakubnowicki

The empty state component that follows the element that should be hidden looks bad.

The documentation is missing.

jakubnowicki · 2023-08-03T13:05:44Z

+  it("checks the empty state component follows slider from id", {
+    app <- shinytest2::AppDriver$new(test_slider_app(), name = "slide_tag")
+    app$expect_values()
+    app$click("toggle_pannel")
+    app$expect_values()
+    app$stop()
+  })
 })


This test is useless.

You have not included the snapshots generated by app$expected_values(), so the tests are not run against the reference, they only generate references.

Even with the snapshots, this test only verifies the values of inputs and outputs, which means that the only thing that you test is if the button was clicked. You are not testing the position of the empty state container.

codecov-commenter · 2023-09-28T20:01:20Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (6b5cf59) to head (cd2f3e3).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main       #42   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           45        45           
=========================================
  Hits            45        45

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jakubnowicki

Please check my comments.

jakubnowicki · 2023-10-18T14:00:56Z

+
+The example below showcases the example of using a dynamic trigger slider that changes.
+
+The function `shiny_emptystate_updatePosition` updates the position of the #container1 after being re-positioned


Please make it clear that this function is 1. JS 2. comes withshiny.emptystate.

jakubnowicki · 2023-10-18T14:02:15Z

  resizeObserver.unobserve(elementToReplace);
 }

+function shiny_emptystate_updatePosition(id){


Please change the name of the function to follow camel case, instead of a weird hybrid. updateElementPosition should work here.

jakubnowicki · 2023-10-18T14:27:01Z

+
+# Introduction
+
+`shiny.emptystate` is capable of being used for dynamic positioning trigger animations. This is specially interesting when we have UIs that change position and we want to keep the `emptystate` in the appropriate tag


Suggested change

`shiny.emptystate` is capable of being used for dynamic positioning trigger animations. This is specially interesting when we have UIs that change position and we want to keep the `emptystate` in the appropriate tag

`shiny.emptystate` is capable of being used for dynamic positioning trigger animations. This is specially useful when we have UIs that change position and we want to keep the `emptystate` in the appropriate tag

jakubnowicki · 2023-10-18T14:28:14Z

+
+# Animation example
+
+The example below showcases the example of using a dynamic trigger slider that changes.


Please explain this in more detail. Also, inform the reader what would happen without the updatePosition function.

jakubnowicki · 2023-10-18T14:29:11Z

-    - icon: fa-file-code-o
+      href: index.html
+    tutorials:
+      text: Tutorials


The new article is not a tutorial, but a small guide. Change this to "Articles".

jakubnowicki · 2023-10-18T14:29:43Z

+
+  it("checks the empty state component follows slider from id", {
+    app <- shinytest2::AppDriver$new(test_slider_app(), name = "slide_tag")
+    position_1 <- app$get_html(selector = "#container2")


This is not a position, but a full HTML. The variable name is misleading.

jakubnowicki · 2023-10-18T14:29:48Z

+    app <- shinytest2::AppDriver$new(test_slider_app(), name = "slide_tag")
+    position_1 <- app$get_html(selector = "#container2")
+    app$click("toggle_pannel")
+    position_2 <- app$get_html(selector = "#container2")


This is not a position, but a full HTML. The variable name is misleading.

jakubnowicki · 2023-10-18T14:31:14Z

+    position_1 <- app$get_html(selector = "#container2")
+    app$click("toggle_pannel")
+    position_2 <- app$get_html(selector = "#container2")
+    expect_false(position_1 == position_2)


As you compare the whole HTML, this may be a false positive (some other parts of HTML will change).

jakubnowicki · 2023-10-18T14:33:14Z

+
+`shiny.emptystate` is capable of being used for dynamic positioning trigger animations. This is specially interesting when we have UIs that change position and we want to keep the `emptystate` in the appropriate tag
+
+**Note:** This solution is only available for instant positioning animation.


Please include the solution for the animations, provided in the issue comments.

Eduardodudu added 2 commits July 31, 2023 22:29

First approach on IntersectionObserver

1190436

Update with manual function

687d808

Eduardodudu added 3 commits August 1, 2023 12:53

update example structure

9a43a60

Add unit test with shinytest2

2aa29cc

Fix lint

d5ddb83

Eduardodudu marked this pull request as ready for review August 2, 2023 23:42

remove bslib from test helper

2aea97f

jakubnowicki requested changes Aug 3, 2023

View reviewed changes

rszymanski mentioned this pull request Aug 28, 2023

[Bug]: Placeholder doesn't follow the tag it covers #32

Open

1 task

Eduardodudu added 2 commits September 28, 2023 11:49

Merge branch 'main' into bugfix/32

dd98767

Add documentation for toggle empty state

b54ace3

Eduardodudu added 2 commits September 28, 2023 17:11

Update reference of gif

b8d7584

Update unit test

cd2f3e3

jakubnowicki requested changes Oct 18, 2023

View reviewed changes


		The example below showcases the example of using a dynamic trigger slider that changes.

		The function `shiny_emptystate_updatePosition` updates the position of the #container1 after being re-positioned


		# Introduction

		`shiny.emptystate` is capable of being used for dynamic positioning trigger animations. This is specially interesting when we have UIs that change position and we want to keep the `emptystate` in the appropriate tag


		# Animation example

		The example below showcases the example of using a dynamic trigger slider that changes.


		`shiny.emptystate` is capable of being used for dynamic positioning trigger animations. This is specially interesting when we have UIs that change position and we want to keep the `emptystate` in the appropriate tag

		Note: This solution is only available for instant positioning animation.

Conversation

Eduardodudu commented Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes description

Uh oh!

Eduardodudu commented Aug 1, 2023

Approaches I tried before this bugfix

IntersectionObserver with getBoundingClientRect

Using MutationObserver

Uh oh!

jakubnowicki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jakubnowicki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Eduardodudu commented Aug 1, 2023 •

edited

Loading

`IntersectionObserver` with `getBoundingClientRect`

codecov-commenter commented Sep 28, 2023 •

edited

Loading