UI formula bar - Githubissues

amy-hoangg commented 3 months ago

Add a formula container inside dom. This container has a selected cell display, fx label and formula input.
Note that the formula bar is connected with only a single cell.
The formula bar has not been able to display math formula

Fluglow commented 3 months ago

I made changes to allow modifying the formula (rawValue) in the formula bar here: https://github.com/lightsheet-team/lightsheet/compare/ui-formula-bar...ui-formula-bar-rawvalue

The UI treats all input as raw values and depends on core events to resolve them and set the actual cell values. This required removing the isReady locking
The formula is stored in the cell elements with setAttribute

Let me know what you think of these changes; maybe we can include this in the PR.

rafinutshaw commented 3 months ago

I made changes to allow modifying the formula (rawValue) in the formula bar here: ui-formula-bar...ui-formula-bar-rawvalue

The UI treats all input as raw values and depends on core events to resolve them and set the actual cell values. This required removing the isReady locking

The formula is stored in the cell elements with setAttribute

Let me know what you think of these changes; maybe we can include this in the PR.

Yes this is how it should be but the ui now breaks is the emply table scenario. dont pass any data in the table and then try to input into an empty cell, new rows start to get added. @Fluglow

rafinutshaw commented 3 months ago

@amy-hoangg also dont shw the formula bar when its in Readonly mode

amy-hoangg commented 3 months ago

In this issue, my first intention is to:

make only a placeholder for selectedCellDisplay and move the functionality of it should be moved to a new issue
move the functionality of displaying raw data to a new issue So you guys think I should include these 2 things in this issue? @Fluglow @rafinutshaw ?

Fluglow commented 3 months ago

dont pass any data in the table and then try to input into an empty cell, new rows start to get added.

This should be fixed in the branch now; I added default ID logic to addRow. @rafinutshaw

Fluglow commented 3 months ago

So you guys think I should include these 2 things in this issue?

At least for the second point, I think we should include editing the formula in this PR already. The changes required for it are fairly minor. @amy-hoangg

amy-hoangg commented 3 months ago

I have an error "RangeError: Maximum call stack size exceeded" when I run the formula bar test and haven't been able to fix. Could @Fluglow or @rafinutshaw help me with that?

rafinutshaw commented 3 months ago

I have an error "RangeError: Maximum call stack size exceeded" when I run the formula bar test and haven't been able to fix. Could @Fluglow or @rafinutshaw help me with that?

this is happening due the an event callback loop. Lemme see if i can figure this out. but it only started occuring after removing the onready lock

Fluglow commented 3 months ago

The loop happens because addCell is firing an event when called from onCoreSetCell to set the passed rawValue. We never use addCell for setting a cell value though since that is done using Sheet.setCell now. Because of that, I'd suggest removing rawValue and columnKey from the parameters and deleting these lines: https://github.com/lightsheet-team/lightsheet/blob/9f647fd7e6c1f74414aa26d559cde7eebd3f8db6/src/ui/render.ts#L213-L216

Fluglow commented 3 months ago

There's one inconsistency I noticed while testing. Since the formula bar is calling onUICellValueChange on input event, the cell value is resolved every time input changes. This means that references are updated as well, while in Excel changes aren't applied until the user presses enter. This is also what happens when you edit a cell's content in Lightsheet, not using the formula bar. I've attached a video to illustrate the difference.

https://github.com/lightsheet-team/lightsheet/assets/38987331/9918dfd1-8572-4f4d-8698-43ed9f06cc6f

amy-hoangg commented 3 months ago

@Fluglow could you check my latest commit that fix the inconsistency?

Fluglow commented 3 months ago

@Fluglow could you check my latest commit that fix the inconsistency?

Works fine now, though shouldn't pressing enter unfocus the input field?

Fluglow commented 3 months ago

I noticed another minor inconsistency when enter isn't used to apply changes. If a cell's formula is modified in the formula bar and another cell is selected (unfocusing the formula input), the changes made to the formula are not applied. If the cell's formula is modified using the cell input field, the change is applied.

https://github.com/lightsheet-team/lightsheet/assets/38987331/7df92ce3-2107-4b33-9b4a-4fd4e750f9c6

amy-hoangg commented 3 months ago

@Fluglow thanks for noticing this error! I have added a new condition to fix it. Could you see whether it works or not?

lightsheet-team / lightsheet

UI formula bar #111