Skip to content
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

Refactor Pathways system #717

Merged
merged 15 commits into from
Apr 10, 2022
Merged

Refactor Pathways system #717

merged 15 commits into from
Apr 10, 2022

Conversation

MiccWan
Copy link
Collaborator

@MiccWan MiccWan commented Apr 8, 2022

  • add PathwaysSession that allows users to record their own projects
  • move PDP inside PathwaysSession
  • clean up PathwaysView

#712

@MiccWan MiccWan requested a review from Domiii April 8, 2022 09:50
@@ -80,6 +81,7 @@ export default class ExerciseIntroductionView extends WebviewWrapper {
}

export async function showExerciseIntroduction(exercise) {
showExerciseIntroductionView(exercise);
Copy link
Owner

Choose a reason for hiding this comment

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

@MiccWan
this one does not have emit in its name, but all others do?

@@ -78,6 +79,7 @@ export default class ValueTDSimpleNode extends ValueNode {
}

valueRender() {
emitValueRenderAction(this.value, this.nodeId);
Copy link
Owner

Choose a reason for hiding this comment

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

@MiccWan
Good rule of thumb: either place all events before or place all events after the thing happened. Don't ever switch orders on that. Since we are mostly calling emit afterwards, let's try to make sure, we keep that order

@@ -57,10 +58,12 @@ let graphWebView;

export async function showGraphView() {
await initGraphView();
emitCallGraphAction(UserActionType.CallGraphVisibilityChanged, { isShowing: true });
Copy link
Owner

Choose a reason for hiding this comment

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

order

@@ -55,11 +57,13 @@ export default class PathwaysWebView extends RichWebView {
let pathwaysWebView;

export async function showPathwaysView() {
emitPathwaysAction(UserActionType.PathwaysVisibilityChanged, { isShowing: true });
Copy link
Owner

Choose a reason for hiding this comment

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

order (and there is more)

@Domiii Domiii merged commit 3a74eef into master Apr 10, 2022
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.

2 participants