Skip to content

Conversation

@mikkurogue
Copy link

Implement simple cell editing that edits the record from the datatable data in place. Provide the onEdit callback to allow hooking external logic to the featureset.

This is mostly a small version of the grander feature as has been discussed at length in some issues where a workaround was suggested to use the render function of the column. However, this appears to be very cumbersome to do and maintain. In this case, I belive that I speak for others too, that this may be a good core feature of the datatable with room for expansion in the future.

This addresses the issue #610 but I don't believe this would close it fully.

I wonder what else there may be that is missing for the first iteration/basic version of this featureset.

Let me know and I'll try to make some time to implement or fix anything new.

I have added an example page to demonstrate it with the basic implementation, this is insipired by but not really the same thing as my provided snippet in the linked issue.

icflorescu and others added 4 commits November 7, 2025 18:38
Implement simple cell editing that edits the record from the datatable
data in place. Provide the onEdit callback to allow hooking external
logic to the featureset.

This is mostly a small version of the grander feature as has been
discussed at length in some issues where a workaround was suggested to
use the render function of the column. However, this appears to be very
cumbersome to do and maintain. In this case, I belive that I speak for
others too, that this may be a good core feature of the datatable with
room for expansion in the future.
@codesandbox
Copy link

codesandbox bot commented Nov 12, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@icflorescu icflorescu changed the base branch from main to next November 17, 2025 14:20
@icflorescu
Copy link
Owner

This looks promising, I must say, but it only works for strings.
I'm pretty much up to my ears in something else and unfortunately can't allocate too much time for it, but I was wondering if we could make it a bit more generic...

@icflorescu icflorescu added the enhancement New feature or request label Nov 17, 2025
@mikkurogue
Copy link
Author

This looks promising, I must say, but it only works for strings. I'm pretty much up to my ears in something else and unfortunately can't allocate too much time for it, but I was wondering if we could make it a bit more generic...

Yeah, currently it's not as flexible as intended but I also didn't have as much time to work on it as I had hoped initially. If it's fine with you, we can keep the PR open as I find some time in the evenings to work on it.

My main goals are still;

  • Implement some form of styling allowance by exposing input props from the specific input types
  • Allow for generic inputs (NumberInput, checkbox/switch for boolean flags and date pickers for date values)
  • Come up with a clean default style that wont cause slight layout shifting

I think once these 3 are (at the very least 90% of the way there) then I'd say its closer to a -rc than current.

@icflorescu
Copy link
Owner

icflorescu commented Nov 17, 2025

My thoughts exactly.

Come up with a clean default style that wont cause slight layout shifting

Yeah, that would be awesome. Perhaps we could use variant="unstyled".

Allow for generic inputs (NumberInput, checkbox/switch for boolean flags and date pickers for date values)

I'm not sure we should go that far. Maybe we should stick to the simple TextInput and find a way to safely cast the value to the underlying data type, nothing more that best effort. Anything more than that would open a fresh can of worms. Think of number inputs and date pickers - those need to be localized in most apps.

@mikkurogue
Copy link
Author

Yeah, that would be awesome. Perhaps we could use variant="unstyled".

Yeah that's what I was thinking, with style props exposed it allows the api to be "flexible" for the end user without being too opinonated but I still want to experiment with this.

I'm not sure we should go that far. Maybe we should stick to the simple TextInput and find a way to safely cast the value to the underlying data type, nothing more that best effort. Anything more than that would open a fresh can of worms. Think of number inputs and date pickers - those need to be localized in most apps.

I agree here, but I do then wonder in what way do we want to support casting? A property for the column type? Something akin to

/**
* If true, the cells in this column will be editable.
*/
editable: true;
/**
* Callback fired when a cell in this column is edited.
* Receives the edited record and its index as arguments.
*/
onEdit: (record: T, index: number) => void;
/**
* Type of input to use when editing cells in this column.
* @default 'string'
*/
colType?: 'string' | 'number' | 'date' | 'boolean'; // unsure about booleans tbh doesnt feel like it would improve much in terms of the editing experience?

Which would be limiting to the supported data types, but I also dont really see this as an issue either honestly. We can also say that we support the basic inputs for strings, numbers and dates but anything else is considered "advanced" and we then recommend to use the render callback directly instead?

I have a working version with number and date inputs (not very neat yet) as I am thinking of localization too, and my only idea now is either to use some props for the inputs to help with passing things like locale, decimal and thousand separators etc. I can provide the current version and see if theres any form of viability for us to use any of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants