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

Persist rest mode toggle across page loads #281

Merged
merged 3 commits into from
Mar 4, 2022

Conversation

kylelaker
Copy link
Contributor

This makes the configuration of "rest mode" an app-level configuration
that is a parameter to the library components. This matches with how the
Google Analytics is set; additionally, it means that the environment
variable should be set when building the app and that this isn't a
decision made when compiling the library (so it doesn't have to be set
in the CI pipeline!).

@kylelaker
Copy link
Contributor Author

To be clear this does nothing on a refresh (we're not storing this value anywhere) but at least doing:

  1. Have a build where the environment variable for REST mode was set
  2. Be on any route
  3. Turn off REST mode
  4. Switch to another route
  5. Notice REST mode is re-enabled

@kylelaker kylelaker requested a review from mikeisen1 March 1, 2022 06:05
@kylelaker kylelaker marked this pull request as draft March 1, 2022 14:11
@kylelaker
Copy link
Contributor Author

This is ready to merge but I've moved it to draft because:

  1. It now has a simple merge conflict
  2. I think it'd conflict even bigger with Add OSCALJsonEditor Component #265 and it'd be better to get that PR in first and rebase this one

This makes the configuration of "rest mode" an app-level configuration
that is a parameter to the library components. This matches with how the
Google Analytics is set; additionally, it means that the environment
variable should be set when building the app and that this isn't a
decision made when compiling the library (so it doesn't have to be set
in the CI pipeline!).
@kylelaker kylelaker force-pushed the feature/persist-rest-mode-config branch from 88eb114 to e6b63ba Compare March 2, 2022 19:17
@kylelaker kylelaker marked this pull request as ready for review March 2, 2022 19:18
@kylelaker
Copy link
Contributor Author

Rebased and ready to go!

@mikeisen1 mikeisen1 self-requested a review March 2, 2022 21:01
example/src/App.js Outdated Show resolved Hide resolved
Co-authored-by: Ray Gauss II <rgauss@easydynamics.com>
@kylelaker
Copy link
Contributor Author

@mikeisen1 I see that you re-requested yourself for review. Is there something in particular here you'd like to see changed or can we go ahead and merge this?

@mikeisen1
Copy link
Contributor

mikeisen1 commented Mar 4, 2022

@mikeisen1 I see that you re-requested yourself for review. Is there something in particular here you'd like to see changed or can we go ahead and merge this?

I re-requested a review after seeing your comments on needing to rebase this PR. But everything looks good to go, so I'll go ahead and merge this.

@mikeisen1 mikeisen1 merged commit 0b8bfd2 into develop Mar 4, 2022
@rgauss rgauss deleted the feature/persist-rest-mode-config branch April 5, 2022 13:59
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

3 participants