Open amyjko opened 9 months ago
Hi @amyjko! I have updated the design specification above. When you have time, can you please take a look? Thanks!
Thanks @lwong121! This is a good start. A few things to think about:
The drop down lists different built-in sequences, but some of those sequence functions actually have inputs. What would the interface for those be? Or is the idea that when chosen, they would need to edit the program to specify inputs, and there wouldn't be user interface controls to modify them? (Making custom interfaces for each would definitely be a lot of work, but there might be ways to generate reasonable interfaces for them based on their input types. But it's something to decide).
it is possible to save previous states, but not something that has a good convention right now. Undo would be a way to get back to them, since all changes are essentially program edits. I think there are also questions about how long that might be saved. For example, if the palette is closed, and then is reopened, would it still remember it? For how long? And then it would forget it for that specific expression if it were deleted in code? The simplest way might be to remember it in the interface itself to allow for switching, but as soon as the interface is closed, it forgets.
Hi! Thanks for the feedback.
I agree that having controls for each input is the best experience; I think the key design challenge here would be how to generalize this. We'll inevitably add more built-in sequences in the future, and they could have inputs of arbitrary complexity. Would we hard code each of them? Or would we have a more general way of specifying how to map those inputs onto controls? I think that's still an open design question.
For the data loss from switching, I wonder if it might make sense to just warn about the data loss? I'm not a huge fan of popups, since people don't read them, but I can imagine some kind of control that asks for confirmation before switching. After all, it can be a lot of work to build a sequence, and to lose it due to accidental drop down change would be devastating. I think the key design question here is what that control would look like, and how would we design it more generically, so that we can use it elsewhere in the interface where there's potential for data loss. For example, we have a button called a "confirm button", which you can see on the profile page for logging out. That's a simple pattern we might mimic for the drop down.
Thanks for the second round of feedback! I have added the updates to the design specification V3.
@lwong121 Overall, it looks good, nice work. A few small things:
The sequence of when the confirmation appears is ambiguous. Does someone click the drop down menu, select something, and then the confirmation button appears, and if it's canceled, then the drop down reverts? Or does the confirm button appear after someone attempts to change the drop down, and then asks for confirmation before they can change it? Both are a bit unconventional.
Would the confirmation only happen for custom sequences, are for all built-in ones? (The built-in ones don't really have data loss).
I can't tell from your sketch if you intended to have a documentation link by the sequence inputs. (The little book icon). Is that just an unintended artifact of your sketch?
One alternative design to the options above is to disable the drop down when custom is chosen and there's some custom sequence. That way, someone would just explicitly press the "x" button to remove the custom one, re-enabling the drop down.
Hi! Thanks for the feedback. Here are my responses:
Thank you @lwong121, you're doing a great job iterating on feedback! I think this is ready for building. I've removed the needs design
label and added the buildable
label. Congratulations on the approved design!
Hi! Just a heads up, I do plan to continue working on this issue next quarter. Thank you!
It's the end of Winter! Please provide an update on this issue, including:
If you do plan to continue work on it, carry on. Otherwise, thank you for your contributions!
Hi! I'm not sure if you saw my previous comment, but I do plan to continue working on this issue next quarter. Thanks!
Thanks @lwong121, I'm grateful to have you continue work on this! (I was just pasting this comment on all issues with assignees, so missed your comment).
Hi @amyjko! I'm currently stuck on trying to figure out how to update the project after the user selects a default sequence from the dropdown. Would you happen to know where I can find an example of how we currently revise a Project when a default sequence is selected? Or could you point me in the right direction? Thanks!
See TextStyleEditor.svelte
for a similar examples. applyStyle
and applyWeight
both use the Projects.revise
function to take one project version and replace it with a new one. The harder part is revising the project. There are some utility functions you'll see, like project.getBindReplacements
, which will replace one function call's bind with a new one. In this case, you'd be replacing a Sequence's map input, which has all of the mappings from percent to Pose
, with a new map, or an Evaluate
node that calls the pre-defined sequence.
Programs are tree data structures, composed of nodes with children, so that's what's happening in all of these; you're modifying the program by replacing one node with another node.
Let me know if I can help translate any of that! There's a lot to learn about how programing language compilers work that I'm happy to explain.
Thanks! I looked at the code in those files you mentioned and several related files. I think I understand a bit more about how I could approach this, but I'll definitely need help translating this over and understanding the code.
In SequenceEditor.svelte
, once the user selects one of the default sequences from the dropdown, in my event handler, I can replace the project with a new version using Projects.revise()
and revise the project with the project.getBindReplacements()
. As you said, that was definitely the easier part to understand. :)
But I am struggling to understand what to pass into the getBindReplacements
function, what each of the arguments to the function are used for, and which one I should modify to handle adding a new default sequence. I think I might need to create a new OutputProperty
and OutputPropertyValueSet
or somehow modify the original versions for poses in the propertyValues
map so that I can add the new Seqeuence map input or Evaluate
node for the default sequence, but I am not entirely sure.
Any explanations on these points or guidance on how to go about this would be really helpful. Thanks!
@lwong121 That's good progress!
You can see the type signature for getBindReplacements
in Projects.ts
. It takes three arguments: a list of Evaluate
expressions to modify, the name of the input to modify, and the expression the value should have (or undefined if it should be removed from the evaluate expression).
Suppose you had a Sequence
evaluate expression like this that we have stored in a variable seq
:
Sequence()
A call to getBindReplacements([seq], 'poses', parseExpression(
myFancySequence())
)` would make an expression like this:
Sequence(poses: myFancySequence())
Then, you'd replace the existing Sequence
expression with that new one.
You shouldn't have to use OutputProperty
at all; those are just special wrapper classes for inputs to Phrase
, Group
, and Stage
, but aren't relevant to Sequence
.
All of these API calls are really just shortcuts to doing tree manipulation: programs are trees of nodes, and these functions just help replace subtrees with other subtrees.
I'm happy to explain more!
Thanks for clarifying that, it much makes more sense now!
I think I managed to replace the existing Sequence expression every time I click on a different option in the dropdown using the same structure from your example (yay!). So I currently have something like this in SequenceEditor.svelte
:
<script>
...
$: selectedSequenceOption = "custom"; // need to figure out where to store this info
function switchSequenceOption(option: string | undefined) {
const posesProperty = SequenceProperties.find(property => property.getName() === "poses");
if (posesProperty !== undefined) {
const posesOutputs = propertyValues.get(posesProperty);
if (posesOutputs !== undefined && option !== "custom") {
Projects.revise(
project,
project.getBindReplacements(
posesOutputs.getExpressions(),
posesProperty.getName(),
parseExpression(toTokens(`${option}()`)))
);
selectedSequenceOption = option;
}
}
}
</script>
<div class="sequence-properties">
<Options
value={selectedSequenceOption}
label={$locales.get((l) => getFirstName(l.output.Sequence.option.names))}
id="sequence-options"
options={[
...sequenceOptions.map((sequence) => {
return {
value: sequence,
label: sequence,
};
}),
]}
change={(option) => switchSequenceOption(option)}
width="10em"
/>
...
</div>
But now I am trying to figure out where might be the best place to store information on which sequence option (custom or one of the default sequences) is currently selected since this is not a palette property like poses or duration. Would you happen to have any suggestions for this?
That's a good question. The overarching design of the palette is to provide a different view on what's specified in code, and so the code itself is the sole place where information is stored. It should be possible to derive the selected sequence option from the code: you'd get the current selection, figure out of it matches any of the default sequences, and if it doesn't, it would be custom. So you shouldn't need to store any information; the program already specifies what's chosen. (Unless I missed some aspect of the design).
Thanks! I think that could definitely work with my current design, but I haven't been able to figure out how to access the current selection from the code. I tried using several functions from project
(e.g. getOutput()
), but I have not yet had any success. Are there any existing examples from the code for how to do this?
The trick is getting access to the Evaluate
node that represents the Sequence
being created. That contains the poses
input that your code is redefining. Once you have that, you just want to write code that checks the first value of that Evaluate
inputs
field, which has the list of inputs sent. The first input is the map of poses, and so if that input is a MapLiteral
, then that means it has a concrete sequence set. If it's another Evaluate
, then that means it's evaluating a predefined function that returns a map. You can use Evaluate.getFunction
to see what function it's referring to, and see if it matches any of the functions defined in Project.shares.sequences
, which is the list of predefined sequences.
See if that gets you started?
Link to branch with my work so far: https://github.com/wordplaydev/wordplay/tree/288-sequence-dropdown
I will not be able to continue working on this issue moving forward, so I will unassign myself from this issue and let someone else continue with the development of the feature. Thanks for a great quarter!
Thank you for the update @lwong121, unassigning you.
What's the problem?
The palette allows for the creation of custom sequences, but there are many sequences provided by default that can be used, but they aren't discoverable.
What's the design idea?
In addition to custom sequences, add to the
SequenceEditor
component a drop down of pre-defined sequences inDefaultSequences
. Choosing one would change the value to be a call to the sequence creation function.Who benefits?
Anyone trying to quickly create a sequence animation without having to define one by manually.
Design specification
(This section should be included after a design proposal is ready and approved, and the
buildable
tag is added. This text can remain until then. Designers should add their proposal here, not in a comment).Design V1
To start creating a sequence, the user will edit the entering, resting, moving, or exiting property in the palette and click on the “Convert to Sequence” button. At the top of the SequenceEditor component, I will add a drop down with options for DefaultSequences (sway(), bounce(), spin(), fadein(), popup(), shake()) and the option to define a custom sequence. The custom sequence option will be selected by default.
Then once the user selects an option, the project code will be updated with a call to the default sequence function and the palette will be updated with the new properties.
Additional Improvements:
Design V2
Some sequence functions have additional inputs to specify behaviors. For example, sway() has the angle input and bounce() has the height input. So I would need to show a slider to specify the angle if sway() is selected and another slider to specify the height if bounce() is selected. Though this might be more work, but I think this would make it more obvious that they have inputs instead of making users have to edit the code again. Below is my plan for the interface.
Also instead of saving the previous state for custom sequences, we can just rely on saving that information in the interface for now to allow for switching between sequence options.
Design V3
I think one way we can generalize the control for inputs is by creating a sequence input editor that contains all the inputs for a default sequence. Currently the existing built-in sequences with inputs only have one input, but design is more flexible if we ever wanted to have multiple inputs in the future. Then for each input control, we would use a text field instead of the sliders I had in earlier designs so that it can be generalized for other numeric inputs. To do this, we would also need to keep track of the units for each input so that we can be more clear about what values they should input (e.g. m, º).
For the second point in the feedback, I agree. I like the idea of adding a confirm button to warn users about possible data loss and double check that it is something they are ok with the consequences before switching. So I added a control with a generic message and button label that we can use to ask for confirmation in similar situations. It will show up after the user has selected a different sequence option (see the resting property).
Design V4
To prevent data loss when the user creates a custom sequence, we will disable the sequence dropdown whenever “custom” is chosen and add an “x” button next to it. Clicking the “x” button will remove the custom sequence and re-enable the dropdown. This will ensure that users are making a mindful decision to remove their custom sequence before switching to a different one. Default sequences will not require confirmation since there is little to no data loss.
Each of the sequence inputs will also have a documentation link to give users more details on what the input is for. Though the inputs we have now are quite simple, this could be helpful in the future when we have more complex inputs that require more explanation.