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

EditableGridCell should have a common newValue property #5

Closed
dvdsgl opened this issue Dec 24, 2020 · 1 comment · Fixed by #7
Closed

EditableGridCell should have a common newValue property #5

dvdsgl opened this issue Dec 24, 2020 · 1 comment · Fixed by #7
Labels
good first issue Good for newcomers type:enhancement Requests for enhancements or new features

Comments

@dvdsgl
Copy link
Member

dvdsgl commented Dec 24, 2020

type EditableGridCell = TextCell | ImageCell | BooleanCell | MarkdownCell | UriCell | NumberCell;

EditableGridCell does not have a common property across union types that represents the new value, so you have to do clumsy checking in onCellEdited (for example). It would be nice to be able to:

onCellEdited={([col, row], editedCell) => console.log(editedCell.newValue)}
@dvdsgl dvdsgl added enhancement good first issue Good for newcomers labels Dec 24, 2020
@schani
Copy link
Member

schani commented Dec 25, 2020

I think the naming is just bad:

  • Most cells have data, except for BooleanCell, which has checked.
  • Some cells also have editData, which is the actual underlying value, whereas their data is the display value.

I think we should

  • Make data always be the underlying value, and have an optional displayData when appropriate.
  • Rename checked in BooleanCell to data.

This was referenced Dec 26, 2020
schani added a commit that referenced this issue Jan 3, 2021
LukasMasuch added a commit to LukasMasuch/glide-data-grid that referenced this issue Aug 17, 2023
* Fix unit tests

* Fix tests

* Remove min / max
jassmith added a commit that referenced this issue Aug 18, 2023
* DatePickerCell Implementation with prettier

* Suggestions. Note, I do need to add more unit tests

* Actually remove the unit test beacuse infra does not support this as of yet.

* Rename variable to dateKind

* Add support.ts duplicate method into cells packagea and refactor import

* Put back try and catch. oops

* readd format parameter to support backwards compat and incorporate Lukas Feedback

* Adding basic unit test

* Add todos for myself

* lint

* Add max, min, step, readonly, and change cached displayDate behavior to undefined behavior

* Add more tests and dataid for date-picker-cell

* Add tests

* Add date, time, datetime tests

* cleanup

* Remove console.log

* Add more test cases for onPaste

* Add time test case for onPaste

* Add implementation for time in onPaste

* Cleanup tests a bit

* Add additional columns to storybook

* Apply correct formatting

* Apply correct formatting

* Clean up with correct prettier file

* Remove need for support functions

* Clean imports

* Revert "Clean up with correct prettier file"

This reverts commit 32e5b93.

* Add assert method

* Fix paste issue

* Delete support ts

* Fix cell stories display

* Add styling to apply gdg theme

* Fix tests

* Use outside read-only

* Remove theme

* Add cell tests to CI workflow

* Fix tests and remove extra test

* Remove tests because they're failing based on timezone

* Update date picker cell (#5)

* Fix unit tests

* Fix tests

* Remove min / max

* Add support for Date as min / max value (#6)

* Fix unit tests

* Fix tests

* Remove min / max

* Add support for date in min/max

---------

Co-authored-by: lukasmasuch <lukas.masuch@gmail.com>
Co-authored-by: Jason Smith <jassmith@gmail.com>
jassmith added a commit that referenced this issue Aug 19, 2023
* 5.2.2-beta1

* Fix unable to select last col

* Find better react-laag fix

* Fix build error

* Prevent crash

* Ensure click off container properly supports touches

* 5.2.2-beta2

* Improve error messages

* Offer increased control over search

* Add dumb filter example

* Fix import

* Export GetRowThemeCallback

* Ensure canvas is in proper ltr/rtl mode

* Don't accidentally exclude final col when copying rows

* Make sure to support RTL in column headers

* Load up testing framework

* Fix build

* Add row marker stories

* Add more type exports

* Fix padding with wrapped text

* 5.2.2-beta3

* Fix copy pasta discovered during merge

* Bump to 5.2.2-beta5

* feat: pass through middle mouse click event to onCellClicked (#693)

* pass through middle mouse click event to onCellClicked

* Add tests and prevent werid behavior

* Add more tests and fixes

* Oops

* Fix build

---------

Co-authored-by: Jason Smith <jason@heyglide.com>

* Fix issue with missing ts distribution (#673)

* feat: expose the cell location in keyboard event (#664)

* feat: expose the cell location in keyboard event

* Add test

* Fix API

---------

Co-authored-by: Alex Corrado <alex.corrado@heyglide.com>
Co-authored-by: Jason Smith <jason@heyglide.com>

* DatePickerCell Implementation (#627)

* DatePickerCell Implementation with prettier

* Suggestions. Note, I do need to add more unit tests

* Actually remove the unit test beacuse infra does not support this as of yet.

* Rename variable to dateKind

* Add support.ts duplicate method into cells packagea and refactor import

* Put back try and catch. oops

* readd format parameter to support backwards compat and incorporate Lukas Feedback

* Adding basic unit test

* Add todos for myself

* lint

* Add max, min, step, readonly, and change cached displayDate behavior to undefined behavior

* Add more tests and dataid for date-picker-cell

* Add tests

* Add date, time, datetime tests

* cleanup

* Remove console.log

* Add more test cases for onPaste

* Add time test case for onPaste

* Add implementation for time in onPaste

* Cleanup tests a bit

* Add additional columns to storybook

* Apply correct formatting

* Apply correct formatting

* Clean up with correct prettier file

* Remove need for support functions

* Clean imports

* Revert "Clean up with correct prettier file"

This reverts commit 32e5b93.

* Add assert method

* Fix paste issue

* Delete support ts

* Fix cell stories display

* Add styling to apply gdg theme

* Fix tests

* Use outside read-only

* Remove theme

* Add cell tests to CI workflow

* Fix tests and remove extra test

* Remove tests because they're failing based on timezone

* Update date picker cell (#5)

* Fix unit tests

* Fix tests

* Remove min / max

* Add support for Date as min / max value (#6)

* Fix unit tests

* Fix tests

* Remove min / max

* Add support for date in min/max

---------

Co-authored-by: lukasmasuch <lukas.masuch@gmail.com>
Co-authored-by: Jason Smith <jassmith@gmail.com>

* 5.2.2-beta6

* Add obstructed playground

* Cleanup events a little more

* Bump to 5.3.0

---------

Co-authored-by: Alex Corrado <alex.corrado@heyglide.com>
Co-authored-by: Caleb Hoyoul Kang <caleb.kang@hpe.com>
Co-authored-by: Lukas Masuch <Lukas.Masuch@gmail.com>
Co-authored-by: Vigen <95758873+vabrahamyanadobe@users.noreply.github.com>
Co-authored-by: willhuang1997 <willhuang1997@gmail.com>
@LukasMasuch LukasMasuch added type:enhancement Requests for enhancements or new features and removed enhancement labels Dec 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type:enhancement Requests for enhancements or new features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants