Open mavcook opened 1 year ago
Thank you! It's a great implementation. Since you have two commits that somewhat overlap, it might be easier to see the detailed suggestions here in this temporary branch (some are just for suggesting an alternative name and a few are changes to the parts that where not part of your commits):
https://github.com/muonw/muonw-powertable/commit/724ef81061336a7be93164a99cff54301f5ba54d
The only major change is the removal of the Cell
interface even though it makes the code cleaner (you will find a comment on the reason for this).
Nice, thank you for the review and changes! It is cool learning more about javascript and Svelte performance.
It looks like you made a lot of the changes in your suggestions commit, so I resolved the related conversations. I will pull your commit into my branch, and I will work on the remaining changes.
Hi @muonw-public, all previous feedback has been addressed, so this PR should be good to land when you get a chance to do a final check. Let me know if there is anything else you would like to see, or if I can help in any way. Thanks!
It's unclear how to preserve the integrity of data with this change. In particular, if we have props: {selectValues: ['Red', 'Blue', 'Green']}
in the instructs, but data already contains the value 'Yellow', how should that be handled during the edit.
Regardless, it makes sense to continue waiting for the release of Svelte 5 as it may call for a rewrite.
I took a stab at resolving #29, which allows users to use a custom component when editing a cell in the table. My approach was to create an object for
edit
related configuration in theInstructs
interface. This gives a user almost full control over how they want editing to work, and leaves room for more capabilities and tuning in the future.Check the updated
example7/
code for full context, but below gives a quick idea of the end result.We add a custom component for the
favoriteColor
column that is a dropdown menuChanges
EditBlock
component encapsulates the default textarea edit behaviorClient/User Breakage
rowClicked
eventB2. Clients usingdataComponent
s will get warning<component> was created with unknown prop 'instructs'
(this can be easily reverted)Notes
N0. This is a larger change than initially anticipated, and as such it introduces slightly different behavior. If the change are too much, or not the right approach, I am happy to rework anything.
N1.
Adding theCell
interface made documentation and user adoption of the edit component a bit easier, but maybe isn't the right naming or approach. I didn't see any way to inhertance or templating in Sveltefixes #29