Nimble break point ixd spec

[aangerhofer]

Pull Request

🤨 Rationale

Breakpoints are needed for debugging workflows, this is an ixd spec for them

👩‍💻 Implementation

IXD spec to begin addressing the problem

🧪 Testing

N/A

✅ Checklist

• I have updated the project documentation to reflect my changes or determined no changes are needed.

[jattasNI]

FYI @aangerhofer there are a couple threads that didn't you didn't respond to (probably because GitHub UI likes to hide comments when a reviewer like me leaves too many of them)

You can see them by clicking Load More or viewing the comments from the Files Changed tab

[jattasNI]

I left a couple more comments on the unresolved threads

[jattasNI]

This was my understanding of the discussion from our meeting last week, does that sound right to you?

Suggested change

	By default, The breakpoint can be added to any row in the table. Implementers of the table should decide if certain rows should not have breakpoints.
	By default, The breakpoint can be added to any data row in the table (including hierarchy rows and leaf rows but not including grouping rows). Implementers of the table should decide if certain rows should not have breakpoints.

[aangerhofer]

My understanding was that it should be added to grouping rows as well. Ex. Putting a breakpoint on "Cleanup" to break before the cleanup operation. The only place I would not expect it is column headers.

[aangerhofer]

lmk if my changes to this are what you are imagining

[jattasNI]

The doc captures the desired state now, thanks.

A word of warning: surprisingly, the work to add breakpoints on grouping rows will probably be much higher than the work to add them on hierarchy and leaf rows. This is because the group rows are computed by the table rather than being provided by the app when they set the table data. So to allow apps / users to set breakpoints on group rows will mean a whole new mechanism to specify info at the group level. We haven't brainstormed what that would look like yet but it feels like a fairly big refactor.

I'd strongly encourage trying to find ways to avoid doing that work. Some possibilities:

1. scope it out for now and see if anyone complains
2. consider alternate UX. For example:
   • not using grouping and instead using hierarchy to represent the categories like "Cleanup"
   • adding a placeholder data row after each grouping row and allowing someone to set a breakpoint on that row, but not the grouping row.

[jattasNI]

Mostly just formatting cleanup comments in this round, though I left a couple more feature clarifications too.

I will check with the other reviewers to see who wants to review and who's going to opt out. But I think we are very close now!

[jattasNI]

nitpick: this makes it sound like they rejected the idea, but I suspect it's rather that they haven't thought about it yet? Also, in the rendered markdown this is running together on one line so I changed the formatting.

Suggested change

	> Note: Not approved by visual design
	Breakpoints should appear to the left of checkboxes for multiselect
	> Note, not yet approved by visual design: Breakpoints should appear to the left of checkboxes for multiselect

[jattasNI]

This section is still showing the template instructions. The required configuration is mostly covered below so maybe something like this would capture what we need.

Suggested change

	*What types of options need to be available on the component to support client-user use cases?*
	An application should be able to configure a breakpoint in any row in any state programmatically. See below for interaction details.

[jattasNI]

What does "optionally" refer to here?

1. This may or may not be in scope?
2. An app can decide whether or not to turn on highlighting?
3. A user can decide somehow? (if so, how?)

If it's 2 or 3, we will need some visual design work. Is this the same as one of the existing row states, like hover, selected, or keyboard focused? Or is it a new state?

We don't need to decide what the visual design is in this PR but it would be good to clarify whether it's a new state or not. For simplicity's sake, I'd hope to just use an existing state, in which case let's change this language to say which one.

[jattasNI]

Suggested change

	- Click on the breakpoint indicator toggles its state (set/remove). The breakpoint itself will likely be a 12x12 vector in a 16x16 icon. The hit target should be a minimum of 24x24 pixels for web use. Mobile use is not a current requirement but this should expand to 48x48 for touch if it ever becomes one.
	- Clicking on the breakpoint indicator toggles its state (set/remove). The breakpoint itself will likely be a 12x12 vector in a 16x16 icon. The hit target should be a minimum of 24x24 pixels for web use. Mobile use is not a current requirement but this should expand to 48x48 for touch if it ever becomes one.

[jattasNI]

Are we ok with using the row action menu for each of these that are listed as "right-click"? In general we've preferred this as right-click on the web is a less common gesture (usually it shows a browser menu, not an application menu) and it takes more work to implement.

If we're not sure yet or if it's more of a scoping question than a behavior question, I'd be fine to remove "right-click" and just say "context menu" for now.

[jattasNI]

To be a bit more pointed about this, after chatting with Milan I realized that right click would be a lot more work to implement so I'd advocate to start by scoping it out and just relying on existing row context menus.

[jattasNI]

Do you know at this point whether the tooltip needs to be different per row or if it's the same for every row? Or if it will change depending on the breakpoint state?

Not critical to resolve now, but we'll need to know when we get to the development spec.

[jattasNI]

This formatting looks a little off in the rendered markdown, with some of these headers showing up as bullets in the list above. I think this cleans it up but feel free to tweak.

Suggested change

	- **Keyboard Shortcuts:**
	**Navigating the table with breakpoints**
	The breakpoint cell will be within the table so navigation should be the same as existing table navigation.
	- **Add/Remove breakpoint:**
	By default, The breakpoint can be added to any data row in the table (including hierarchy rows and leaf rows but not including the table header row). Implementers of the table should decide if certain rows should not have breakpoints.
	When the user is focused on the breakpoint cell Space/Eneter adds the breakpoint in addition to the beyboard shortcuts below
	When the user is focused on table row or group header only the keyboard shortcuts add the breakpoint.
	#### Keyboard Shortcuts
	**Navigating the table with breakpoints**
	The breakpoint cell will be within the table so navigation should be the same as existing table navigation.
	**Add/Remove breakpoint:**
	By default, The breakpoint can be added to any data row in the table (including hierarchy rows and leaf rows but not including the table header row). Implementers of the table should decide if certain rows should not have breakpoints.
	When the user is focused on the breakpoint cell Space/Enter adds the breakpoint in addition to the keyboard shortcuts below.
	When the user is focused on table row or group header only the keyboard shortcuts add the breakpoint.

[jattasNI]

This section feels out of place here, it seems like it's in the middle of keyboard shortcuts section but it applies to more than just keyboard. Maybe move up to the Configuration section where I left a related comment?

[jattasNI]

The PR pipeline is failing in the lint step (see the "Checks" section at the bottom of the Conversation tab). This is an automated check that makes sure our documents have consistent formatting. To fix it you can follow these steps in CONTRIBUTING.md. If you haven't already, you'll first have to do the getting started steps (install Node.js and run npm install).

I recognize that's a lot just to submit a markdown document so I'd be happy to run those steps for you once all the comments are resolved if you prefer.

[jattasNI]

The doc captures the desired state now, thanks.

A word of warning: surprisingly, the work to add breakpoints on grouping rows will probably be much higher than the work to add them on hierarchy and leaf rows. This is because the group rows are computed by the table rather than being provided by the app when they set the table data. So to allow apps / users to set breakpoints on group rows will mean a whole new mechanism to specify info at the group level. We haven't brainstormed what that would look like yet but it feels like a fairly big refactor.

I'd strongly encourage trying to find ways to avoid doing that work. Some possibilities:

1. scope it out for now and see if anyone complains
2. consider alternate UX. For example:
   • not using grouping and instead using hierarchy to represent the categories like "Cleanup"
   • adding a placeholder data row after each grouping row and allowing someone to set a breakpoint on that row, but not the grouping row.

[jattasNI]

To be a bit more pointed about this, after chatting with Milan I realized that right click would be a lot more work to implement so I'd advocate to start by scoping it out and just relying on existing row context menus.