Skip to content

feat(mode): Remove toggle logic#590

Merged
mergify[bot] merged 2 commits intobox:masterfrom
mingzexiao6:remove-toggle
Sep 15, 2020
Merged

feat(mode): Remove toggle logic#590
mergify[bot] merged 2 commits intobox:masterfrom
mingzexiao6:remove-toggle

Conversation

@mingzexiao6
Copy link
Contributor

The FSM in Preview SDK maintains the source of truth. So there is no need to have extra logic for setting mode in annotations.

@mingzexiao6 mingzexiao6 requested a review from a team as a code owner September 14, 2020 23:26
builder.addCase(toggleAnnotationModeAction, (state, { payload: mode }: { payload: Mode }) =>
state === mode ? Mode.NONE : mode,
),
builder.addCase(toggleAnnotationModeAction, (state, { payload: mode }: { payload: Mode }) => mode),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems a bit risky, since the Preview SDK and Annotations SDK may not be released at the same time. Should we create a setAnnotationMode event or method, instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Preview SDK changes are behind the discoverability feature flip, so Annotations SDK will release first, which is not risky, because all the usages in Preview SDK is already using toggleAnnotationMode as setAnnotationMode.

@mergify mergify bot merged commit 63f1cfb into box:master Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants