Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gantt: Request for improved API for custom cell editors #7267

Closed
FlowIT-JIT opened this issue Aug 4, 2023 · 15 comments
Closed

Gantt: Request for improved API for custom cell editors #7267

FlowIT-JIT opened this issue Aug 4, 2023 · 15 comments
Assignees
Labels
feature request high-priority Urgent to have fixed resolved Fixed but not yet released (available in the nightly builds)
Milestone

Comments

@FlowIT-JIT
Copy link

FlowIT-JIT commented Aug 4, 2023

Hi,

We use Gantt's ability to display custom cell editors for data manipulation. The API that allows for this is quite simple - something along the lines of:

render(): React.Component
getValue(): any
setValue(val: any)
isValid(): boolean
focus()

Some of our controls open widgets that are mounted outside the table cell - e.g. dialogs, calendar widgets, pull down menus, etc. Interacting with these DOM elements result in gantt exiting edit mode. We have been able to get it to work by added control specific logic outside of the React control using gantt's beforeComplete event which detects the presence of various widgets in the DOM and then prevents gantt from exiting edit mode by returning false. But that's a hack, and having to implement this outside of our custom React cell editors is really not desirable nor logic.

All our controls know exactly when they are done, which is when they lose focus (and they don't lose focus when interacting with pull down menus, dialogs, calendar widgets, etc.). So what we are requesting is the ability to ask gantt not to handle "commits" for us, but let us do it manually instead. Example of such implementation:

export default class ControlWrapper extends React.Component<ControlWrapperProps, ControlWrapperState> {
    private control: ControlInstance | null = null

    constructor(props: ControlWrapperProps) {
        super(props)
        this.state = {
            value: undefined,
            isValid: true,
            columnId: undefined,
        }

        // Feature request:
        // Tell Gantt to NOT auto commit when cell lose focus or when
        // pressing ENTER, and don't exit/cancel when pressing ESC either.
        this.manuallyCommit(true)
    }

    render() {
        const onCreate = (control: ControlInstance) => {
            this.control = control

            control.onChange((value, isValid) => {
                if (this.props.controlType === "calendar") {
                    this.setState({ value: new Date(value), isValid: isValid })
                } else if (this.props.controlType === "integer") {
                    this.setState({ value: parseInt(value), isValid: isValid })
                } else if (this.props.controlType === "decimal") {
                    this.setState({ value: parseFloat(value), isValid: isValid })
                } else {
                    this.setState({ value: value, isValid: isValid })
                }
            })
            control.onBlur(() => {
                if (this.state.isValid) {
                    this.commitChange() // Feature request: Make gantt exit editing mode and call getValue() to persist change
                }
            })
            control.getElement().addEventListener("keydown", (e) => {
                if (e.key === "Escape") {
                    this.cancelChange() // Feature request: Cancel change - make gantt exit editing mode WITHOUT calling getValue() and WITHOUT persisting change
                }
            })
        }

        return (
            <div style={{ padding: "0.6em", color: "black", width: "100%" }}>
                {this.props.controlType === "userpicker" ? (
                    <UserPicker onCreate={onCreate}></UserPicker>
                ) : this.props.controlType === "calendar" ? (
                    <Calendar onCreate={onCreate}></Calendar>
                ) : (
                    <Input onCreate={onCreate} regex={this.props.controlType === "integer" ? /^\d*$/ : this.props.controlType === "decimal" ? /^\d*(\.\d+)?$/ : undefined}></Input>
                )}
            </div>
        )
    }

    // Bryntum cell editing interface - required functions:

    getValue() {
        return this.state.value
    }

    setValue(value: Date | string | number | undefined, cellEditorContext?: { columnId: string }) {
        this.setState({ value: value, columnId: cellEditorContext?.columnId })
    }

    isValid() {
        return this.state.isValid
    }

    focus() {
        this.control?.focus()
    }
}
@matsbryntse
Copy link
Member

Slightly related: #73

@FlowIT-JIT
Copy link
Author

Slightly related: #73

Yes, we had to create a hack for that too, to allow controls to exceed the boundaries of the table cells.

@FlowIT-JIT
Copy link
Author

@matsbryntse Is this feature request something you can commit to and provide an estimate for expected availability?
I'm not too happy with out current implementation. It's not very robust and difficult for other developers to maintain.

@matsbryntse
Copy link
Member

Thanks for your feedback, we'll try to address this later in Q3 or early Q4.

@matsbryntse matsbryntse added the high-priority Urgent to have fixed label Aug 7, 2023
@FlowIT-JIT
Copy link
Author

Much appreciated - thank you 🙏😊

@jsakalos
Copy link

Hello @FlowIT-JIT, I'm working on this ticket and I would like to work with the same code as you do so would you please post your implementations of Input? Possibly also UserPicker and Calendar? And anything else that could be relevant/useful? Ideally in the form of runnable showcase.

@FlowIT-JIT
Copy link
Author

@jsakalos Great! I'll create some examples for you as soon as possible. I might be able to start working on it today.

@FlowIT-JIT
Copy link
Author

FlowIT-JIT commented Nov 29, 2023

@jsakalos I have attached a runnable example to this comment Gantt-5.6.2-react-typescript-basic.zip

It is not identical to what we have running, since it's a huge and very complex app, but it demonstrates the problems we are experiencing with the existing interface. The controls used for this demo are the ones we use.

Thank you for involving us like this 👍

@jsakalos
Copy link

jsakalos commented Dec 15, 2023

Hello @FlowIT-JIT, it finally breaks down to the focus management. The difference between Bryntum combo and your Color selector is that Bryntum combo stays in the editing mode while your Color component does not. The same is true is for your Date picker.

The blur is caused by line 105 of GenericControl.tsx. When I comment it out:

    private exitEditingMode() {
        if (this.control?.IsValid()) {
            // Feature request: Make gantt exit editing mode and call getValue() to persist change
            // this.commitChange()
            // Work around:
            // this.control.Focused(false);
        }
    }

it seems working to me. Let us please know whether this is the behavior you are after.

react-editor

@FlowIT-JIT
Copy link
Author

FlowIT-JIT commented Jan 2, 2024

@jsakalos Hi - happy new year 😊

Yes, I'm aware. The point is, the current implementation does not allow us to focus anything outside of the cell hosting the control. That greatly limits what we can do without applying ugly hacks. We have very complex controls, some of which opens entirely new editing experiences "on top" of Gantt.
The line you suggest disabling is intended to force focus away from the control and cell so gantt exits editing mode. Notice the function in which it is found - exitEditingMode().

The date picker works, but only because I have applied a dirty hack. Try to disable this line:
(this.control as any)._internal.ReduceDocumentRootPollution = true

Now reload, open a date picker, and try changing the month or year, or even select a visible date. The date picker's input field disappears because focus is temporarily lost from the cell, and any selection within the date picker is lost as well. Additionally, the calendar widget remains on the screen, as the control malfunctions when it is prematurely unmounted.

@jsakalos
Copy link

jsakalos commented Jan 3, 2024

Hello @FlowIT-JIT. Happy new year to you too.

We have discussed it internally and we consider adding a logic that would allow to configure a custom focus/blur behavior of editors. Stay tune please, it should materialize soon.

@FlowIT-JIT
Copy link
Author

@jsakalos Wonderful - thanks 😊

@isglass isglass closed this as completed Mar 19, 2024
@isglass isglass added resolved Fixed but not yet released (available in the nightly builds) and removed in progress labels Mar 19, 2024
@isglass isglass added this to the 5.6.9 milestone Mar 19, 2024
@FlowIT-JIT
Copy link
Author

@isglass Exciting 😊 Can you please add a link to the documentation for the new API? - Thanks

@jsakalos
Copy link

@FlowIT-JIT the issue is resolved but it has not yet been released. With 5.6.9 out you will find a new subsection in the guide https://bryntum.com/products/grid/docs/guide/Grid/integration/react/guide#using-react-as-cell-editor with links to API docs. Then, in the docs, turn on "Show advanced APIs" to see the new config option managedCellEditing.

@FlowIT-JIT
Copy link
Author

@jsakalos Wonderful - thank you 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request high-priority Urgent to have fixed resolved Fixed but not yet released (available in the nightly builds)
Projects
None yet
Development

No branches or pull requests

4 participants