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

Remember page state #75

Merged
merged 9 commits into from
Oct 29, 2020
Merged

Remember page state #75

merged 9 commits into from
Oct 29, 2020

Conversation

krystian8207
Copy link
Contributor

@krystian8207 krystian8207 commented Sep 28, 2020

Previously all the pages were initially hidden and shown on bookmark change.
This solution have an important disadvantage:

  • hidden page inputs are not bind,
  • you need to use suspendWhenHidden = FALSE for all the inputs in order to update them.
    We decided to hide all the pages with visibility: hidden.
    This forces all the pages to be rendered during runtime, but between page state is remembered and not rendered again.

What's more the API is more consistent now. You should use router$ui instead of router_ui() and router$server(input, output, session) instead of router(input, output, session).

Copy link
Contributor

@dokato dokato left a comment

Choose a reason for hiding this comment

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

I really like these changes. They solve a lot of issues. Please address outstanding minor comments.

Also, from my tests on internal server, looks like it solves #68. I was thinking of introducing backward compatibility with router and router_ui calls, but I can see that this would be difficult in the current setting, so maybe we can ignore.

R/router.R Show resolved Hide resolved
R/router.R Show resolved Hide resolved
R/router.R Show resolved Hide resolved
Copy link
Contributor

@dokato dokato left a comment

Choose a reason for hiding this comment

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

I tested locally and on R Connect, looks good now 👍

@dokato dokato merged commit 90bfc1c into master Oct 29, 2020
@scizmeli scizmeli deleted the krystian.remember-page-state branch June 29, 2022 11:32
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.

None yet

2 participants