-
Notifications
You must be signed in to change notification settings - Fork 0
Vps 137/Create State Variables #286
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
Open
RLee64
wants to merge
16
commits into
master
Choose a base branch
from
VPS-137/set_state_variables
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
State variables should are shared across a scenario, so this feel like the most fitting place to include them. I've had to change a bit of naming to make things work though (hopefully that's okay)
The styling present is just a temporary solution since we'll be changing things (and borrowed from the Material UI Modal example)
A lot of code here, but it's all about making some sort of interface for users to create state variables with. Currently we've got three state types - String, number, and boolean. Note that the number type allows for the possibility of floating numbers. The new state types file will probably be used across other state variable files to ensure naming consistency (it works like an enum). As of this commit, state variables don't save (gotta figure out how all that scenario/authoring context stuff works first)
StateVariables should be an array of objects for each scenario
Can now create state variables by calling the new endpoint
Implemented the frontend logic for creating new state variables CreateStateVariable.jsx has the code for sending the API request StateVariableMenu.jsx displays the state variables to the user (currently very crudely and should be updated) Also note that the scenario is reFetch'd when the modal is closed
Boolean values weren't being displayed before
1. Resetting to default value was done improperly, it should be correct now 2. Numbers were being stored as strings, the change on line 129 should hopefully fix this
I keep forgetting I don't have it configured to format on save
Not sure what the status on UI updates is rn, but I thought I might as well quickly make a toast component (I don't think one exists yet??)
I feel so stupid for not noticing this
Turns out currentScenario and reFetch don't actually fetch the stateVariables (nor the role list). Since this PR is already kinda big, I'm just gonna remove it here and work on adding it back in next PR.
We want to allow decimals so Number() makes more sense
damn prettier so picky 🙄
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe the issue
The current flag system is fairly limited with what it can achieve since they can only be true or false. We want to implement a new 'State Variable' system that allows multiple data types to be stored and manipulated by a scenario author.
Describe the solution
This is the first PR of a planned 4-5 pull requests involving the implementation of this system from the authoring side. Here we're concerned entirely with the creation of State Variables, which is achieved through a new state variable menu. This PR is really big, so following the commits should be helpful for context (note that I sometimes have mistakes in my commits that I only manage to find and fix later on).
Summary of changes are as follows
StateVariableMenu.jsx
component as a menu for creating and updating State Variables and their initial valuesCreateStateVariable.jsx
component which handles all the State Variable creation logicStateTypes.jsx
as a sort of enum for state types (more need to be added later as well like arrays and maps)Toast.jsx
as a generic toast component (currently only used by CreateStateVariable, but I plan to leverage it in later PRs as well)OpenStateVariableMenu
as a means of handling whether to show or close the State Variable MenuRisk
Notes
Hopefully this doesn't cause any issues, but I'd like to do the documentation closer to the end (in case some stuff changes while working on the other PRs).
Definition of Done
Reviewed By
Who reviewed your PR - for commit history once merged