Activity Directive Builder#1900
Conversation
|
Any reason not to use the activity form here embedded in a draggable window with a few extra components thrown in on top? |
d91262a to
cd6b8e4
Compare
cd6b8e4 to
157b1cc
Compare
157b1cc to
b060112
Compare
b060112 to
7e23e38
Compare
7e23e38 to
4e9d584
Compare
4e9d584 to
1f30961
Compare
1f30961 to
38304f5
Compare
It seemed like it would be a considerable rework to support it here since we want the validation and editing to happen on a directive that isn't created. The code for validation is more-or-less copy/paste though, so at least I think that can be pulled out to a common function. |
b05e206 to
cb284a8
Compare
| import Parameters from '../parameters/Parameters.svelte'; | ||
| import Draggable from '../timeline/form/TimelineEditor/Draggable.svelte'; | ||
|
|
||
| export let directiveWidth: number = 1000; |
There was a problem hiding this comment.
Maybe just width? Know the other ones say "filterWidth" though hoping at some point we can merge #1828 and make that width more generic if it isn't already included in there,
| } | ||
| const activityDirectiveTags: ActivityDirectiveTagsInsertInput[] = tagsToAdd.map(({ id: tag_id }) => ({ | ||
| directive_id: activityDirective.id, | ||
|
|
There was a problem hiding this comment.
whitespace change not needed
| @@ -760,7 +761,7 @@ | |||
|
|
|||
| const createActivities = () => { | |||
| items.forEach(item => { | |||
There was a problem hiding this comment.
Only one buildDirective can win here so all previous dispatches will be lost. This PR is centered around a single builder window for a single directive and I don't think we want to go near a multi-directive builder here but it does break the previous behavior where a user could drag all activity types on the timeline via the handle to the right of the "Activities (X)" header. I doubt anyone was actually using this feature, it's likely more for testing purposes and it is pretty hidden. Should either directly create all of those directives and skip the builder, open the builder with the first type, or get rid of the handle entirely.
|
|
||
| function onCreateActivityDirective(directive: ActivityDirectiveInsertInput) { | ||
| if ($plan !== null && $plan.model) { | ||
| effects.createActivityDirectivePredefined(directive, $plan, user); |
There was a problem hiding this comment.
Should we await creation of the directive first in case it fails to create? I know a failure toast will appear, just means that if any sort of hiccup happens (unlikely) that user input from builder will be lost. Not sure if it is necessary though.
| > | ||
| <UploadIcon /> | ||
| </button> | ||
| <Button variant="ghost" size="icon-sm" aria-label="Add Activity" on:click={onShowDirectiveBuilder}> |
There was a problem hiding this comment.
Button needs permission gating.
| > | ||
| <UploadIcon /> | ||
| </button> | ||
| <Button variant="ghost" size="icon-sm" aria-label="Add Activity" on:click={onShowDirectiveBuilder}> |
There was a problem hiding this comment.
Could this be an outline button that matches the height of the neighboring buttons?
| </div> | ||
| </div> | ||
| <div class="directive-section-content directive-section-content-bordered"> | ||
| <DatePickerField |
There was a problem hiding this comment.
Would be nice to use plan start/end instead of today for the buttons here (think we do it in other places in the app) to make it easier for users to get back to a relevant time range.
| {/if} | ||
| </div> | ||
| </div> | ||
| <!-- </CssGrid> --> |
| let startTimeField: FieldStore<string>; | ||
| let startTime: string = $plan?.start_time_doy ?? ''; | ||
|
|
||
| let currentActivityTypeFormParams: FormParameter[] = []; |
There was a problem hiding this comment.
Can you alphabetize your let declarations in this component?
| } | ||
|
|
||
| .body { | ||
| /* background: var(--st-gray-15); */ |
There was a problem hiding this comment.
Can remove this comment
| return result; | ||
| } | ||
|
|
||
| export async function validateArguments( |
There was a problem hiding this comment.
Can you add unit tests for this?
There was a problem hiding this comment.
Can you add some e2e tests (though I'm sure you're working on those) that use the builder via button and drag and drop?
This PR introduces the activity directive builder, modeled from the activity filter builder for timeline editing, to build an activity directive prior to actual creation on the timeline.
The builder can be accessed 3 different ways:
Activity, Resource, Event Typesmenu allows the user to click the '+' button which brings up the builderActivity, Resource, Event TypesThe directive builder allows the user to fill in a name, start time, and parameter values for a directive prior to having it created on the timeline. Validation for parameter values and missing required parameters is included, but the user is still able to create an 'invalid' directive if they wish.
Testing
Create a plan and choose any of the 3 ways from above to access the directive builder. Then, fill out the information in the builder and click 'Create'. The activity directive with all the entered information should appear on the timeline.