Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Conversation

@nyrosmith
Copy link
Contributor

Associated Issue: NONE

@jasonLaster suggested this feature on slack

Summary of Changes

  • renamed contextmenu entry
  • jump to breakpoint location

conditional

@nyrosmith nyrosmith requested a review from flodolo as a code owner September 19, 2017 18:51
accesskey: editConditionKey,
click: () => toggleConditionalBreakpointPanel(breakpoint.location.line)
label: addOrEditConditionLabel,
accesskey: addOrEditConditionKey,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should have two separate labels "Add Condition", "Edit Condition" that we toggle based on whether there is a condition. We do the same thing when you right click on the gutter.

click: () => {
const sourceId = breakpoint.location.sourceId;
const line = breakpoint.location.line;
this.props.selectSource(sourceId, { line });
Copy link
Contributor

Choose a reason for hiding this comment

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

yes! good catch. Perhaps we move this invokation to this action: toggleConditionalBreakpointPanel

@jasonLaster
Copy link
Contributor

screen shot 2017-09-20 at 9 22 53 am

screen shot 2017-09-20 at 9 21 31 am

Looks like a flow issue. and the mochi 💣 . i'll see what's up there

@@ -119,8 +120,14 @@ export function clearHighlightLineRange() {
}

export function toggleConditionalBreakpointPanel(line: null | number) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@nyrosmith the flow issue had to do with the line input
This should fix it export function toggleConditionalBreakpointPanel(line?: number) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @bomsy for looking at it and helping me out!

@nyrosmith
Copy link
Contributor Author

/remind me to finish this PR tomorrow

@nyrosmith
Copy link
Contributor Author

Closing because of #4157

@nyrosmith nyrosmith closed this Sep 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants