pycodestyle
pycodestyle copied to clipboard
PyCQA
Missing E133 test suite and E131 in E12 test suite
Hi, just to mention that for E133 there's no test in the test suite and the test for E131 is located in E12.py
I'll work on this over the weekend. I think it makes sense to add this as built-in functionality since we already have a keydown listener and an activeKeys store. The groups object is a writable with nested writables, so it might not be great DX to do it manually. It'd be something like get(get(graph.groups).selected.nodes). I've made some good progress on a Shelf component that can take in an array of Node components, which I think is something you were sort of working on yourself, so we could pretty easily just enable this feature with a prop.
Wow! Inbuilt functionality will be very much appreciated :)
But might need forwarding as well since we're also maintaining our own nodes store with context specific to our project. Since we render the components like
<Svelvet
TD={false}
minimap
controls
height={innerHeight -
((browser && document.getElementById('workflow-nav')?.offsetHeight) || 0)}
fitView={true}
>
<Trigger />
{#if $builderStore && $builderStore.steps}
{#each Object.entries($builderStore.steps) as [stepId, stepData] (stepId)}
<Step {stepId} {stepData} />
{/each}
{/if}
<ExportGraph bind:exportGraph bind:getCenter />
</Svelvet>
I guess we'll be able to get a prop here or on Node that fires an even we catch and then subsequently duplicate the nodes on our store?
Okay. I think I've got a pretty nice solution. Let me know if you think this works for you.
There's an on:duplicate event on the Node component. It fires whenever the Meta and "D" keys are pressed together AND the node is currently selected. So, if you have ten selected nodes then press Meta + D, a duplicate event will fire on all of them. While doing this, I also added select all functionality via Meta + A.
To test it out, I was doing something like this. I'm guessing, for your use case, you would listen for it on the Step node and then add another entry to your store with the same stepData that is associated with whatever instances of Step fired the duplicate event.
<script>
let customNodeCount = writable(1);
let thisNodeCount = writable(1);
setContext('customNodeCount', customNodeCount);
</script>
<body>
<Svelvet>
{#each { length: $customNodeCount } as _, i}
<CustomNode />
{/each}
{#each { length: $thisNodeCount } as _, i}
<Node on:duplicate={() => $thisNodeCount++} />
{/each}
</Svelvet>
</body>
This is in the latest version along with the other mentioned changes. Let me know how it goes and I'll close out this issue.
On duplicate event is great, but how would one configure it?
For me, the meta + d key opens the bookmark tab, not sure how else to trigger it. Tried ctrl + d as well.
So, our keydown listener is scoped to the graph, not the window (for obvious reasons), but we were only focusing the graph element when interacting with the grid/canvas/map and not the nodes themselves. This has been updated and will be released with the next version.
I also added a modifier prop to the Svelvet component so that you can set key modifier to "ctrl", "alt", "meta" or "shift". This only changes the duplicate and select all commands at the moment. I'll have to think about how to handle other pre-existing shortcuts (like hitting "ctrl" to clear selected nodes) if the modifier is changed from the default.
So, our keydown listener is scoped to the graph, not the window (for obvious reasons), but we were only focusing the graph element when interacting with the grid/canvas/map and not the nodes themselves. This has been updated and will be released with the next version.
I also added a modifier prop to the Svelvet component so that you can set key modifier to "ctrl", "alt", "meta" or "shift". This only changes the duplicate and select all commands at the moment. I'll have to think about how to handle other pre-existing shortcuts (like hitting "ctrl" to clear selected nodes) if the modifier is changed from the default.
Amazing! Will try this out tomorrow (tho i did select the node before CTRL + d as you can see the black borders around the node),
Also, is there a way to allow CTRL + c & CTRL + v as well? I guess if you can forward the key down events on the svelvet component & then users can figure out how to do it on their own.
I maybe wasn't clear. It was a bug. Selecting a Node didn't focus the graph in previous versions, so the key down listener wasn't active. It was required that you interact with the background/grid itself to enable the keydown listener. That's been fixed, but hasn't been released yet.
I don't think forwarding the key events is all that helpful, unfortunately. I'll probably add it as a kind of easter egg feature. It would require developers to interact with the node/graph object itself, which isn't something we can support. It's there if you need it, but we need to have the flexibility to change those structures internally without worrying about API issues or documentation.
If we did forward the key events, they'd have to look into the graph object and grab the currently selected nodes and then match that to the associated component to initiate a copy event. But the Node object doesn't know anything about what component it's attached to. You'd have to parse our internal NodeKey and match that to whatever ID you gave the Node, if you gave it one.
Instead, we can use native Svelte functionality to do all that under the hood. It's much easier for us to fire the events on the Nodes we know are selected. Then the event already has the contextual information and developers can just trigger whatever function they want. For common commands like copy, paste, duplicate, select all, etc, we should have built in functionality IMO.