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

Memory Table Editability #110

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

colin-grant-work
Copy link
Contributor

@colin-grant-work colin-grant-work commented Mar 20, 2024

What it does

An alternative implementation of #108 that allows both editing by group (by double clicking on the group) and editing of sub- and supra-group editing by selection and context menu.

How to test

As in #108:

  1. Double click on a data group within the table
  2. Provide a different value
  3. submit with enter or blur the element
  4. Changes should be applied

In addition:

  1. Select text in a data column row.
  2. Use the context menu and select Edit currently selected memory
  3. All words whose text was included in the selection will be converted into a text input.

image

image

image

Review checklist

Reminder for reviewers

@jreineckearm
Copy link
Contributor

Thanks, @colin-grant-work !
I see that this PR builds on top of #108 . I'll have a play with it tomorrow. Could make sense to merge #108 first and then rebase/update this one here. Would this here address any open feedback on the other one?

@jreineckearm
Copy link
Contributor

Actually did a little testing now.... :-)
I like this approach as well. But we need to review how this handles endianess. I see unexpected effects with a Little Endian configuration. So, we probably want to focus on #108 first. Then take a proper look at the endianess handling here.

@colin-grant-work
Copy link
Contributor Author

Yes, because this allows editing that spans groups, which is the unit at which we apply endianness currently, it could end up behaving a bit strangely there.

Would this here address any open feedback on the other one?

This one addresses the early separation of data and UI, so basically the feedback on the data-column.tsx. What remains for @haydar-metin to do is primarily the cleaner handling of the event emission after write (the comments on memory-provider.ts).

@colin-grant-work
Copy link
Contributor Author

Setting as draft for now pending reconciliation with recently merged code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants