Skip to content

Conversation

@pregnantboy
Copy link
Contributor

@pregnantboy pregnantboy commented Nov 30, 2025

Changes

  1. Added a new MrfContextProvider to manage form approval branches
  2. Refactoring the useStepMetadata hook to read context values directly from EditorContext
  3. Simplifying the StepExecutionsToIncludeProvider to use a more structured context object
  4. Improving the way steps are passed to EditorRightDrawer

These changes make the code more maintainable and prepare for future form approval functionality.

Copy link
Contributor Author

pregnantboy commented Nov 30, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@pregnantboy pregnantboy changed the title chore: mostly refactor, add placeholder mrf context [MRF-9] PLU-520: MRF context and refactor Nov 30, 2025
@linear
Copy link

linear bot commented Nov 30, 2025

PLU-520 MRF: Approvals

@pregnantboy pregnantboy force-pushed the mrf/add-mrf-context-and-refactor branch 2 times, most recently from 8ef3dc5 to c27ebd0 Compare December 3, 2025 15:42
@pregnantboy pregnantboy force-pushed the mrf/add-mrf-context-and-refactor branch from c27ebd0 to 3374fde Compare December 3, 2025 15:45
@pregnantboy pregnantboy force-pushed the mrf/mrf-approve-reject-select branch from 453b89b to 317e3d3 Compare December 3, 2025 15:45
@pregnantboy pregnantboy marked this pull request as ready for review December 3, 2025 16:24
@pregnantboy pregnantboy requested a review from a team as a code owner December 3, 2025 16:24
isMobile,
isDrawerOpen,
)
} = useStepMetadata(step, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

if set to true, the drag handle will be shown for the for-each step. just a display issue, as its still not actually draggable

Suggested change
} = useStepMetadata(step, true)
} = useStepMetadata(step, allowReorder)

isNested={isNested}
// only allow reordering if there are more than 1 action steps
allowReorder={nonIfThenActionSteps.length > 1}
allowReorder={true}
Copy link
Contributor

@kevinkim-ogp kevinkim-ogp Dec 9, 2025

Choose a reason for hiding this comment

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

Suggested change
allowReorder={true}
allowReorder={allowReorder}

setting this to always be true will show the drag handle when there is nothing to drag. no real impact, just a display thing.

Image

stepsBeforeGroup: IStep[]
groupedSteps: IStep[][]
showAddButton?: boolean
addButtonProps: {
Copy link
Contributor

@kevinkim-ogp kevinkim-ogp Dec 9, 2025

Choose a reason for hiding this comment

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

Suggested change
addButtonProps: {
allowReorder?: boolean,
addButtonProps: {

stepsBeforeGroup,
groupedSteps,
showAddButton = true,
addButtonProps: {
Copy link
Contributor

@kevinkim-ogp kevinkim-ogp Dec 9, 2025

Choose a reason for hiding this comment

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

Suggested change
addButtonProps: {
allowReorder = true,
addButtonProps: {

isLastStep={position === steps.length}
isNested={isNested}
stepsBeforeGroup={actionStepsBeforeGroup}
groupedSteps={groupedSteps}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
groupedSteps={groupedSteps}
groupedSteps={groupedSteps}
allowReorder={nonIfThenActionSteps.length > 1}

Copy link
Contributor

@kevinkim-ogp kevinkim-ogp left a comment

Choose a reason for hiding this comment

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

lgtm, tested and verified that all functionalities remain!

small thing that we may want to fix is to hide the drag handle when there is only 1 draggable action step. does not affect any functionality, pure UI thing

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.

3 participants