-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Fixing issue with grid not allowing copying external ID #13650
Conversation
…to the way IDs were split/handled in the grid.
…l goes through one location, using a new character to join/split.
@aptkingston would appreciate your input on this - after some discussion with @samwho realised that we just can't use Instead using something very unusual to avoid this problem - |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Technically these are cell IDs rather than row IDs so a rename would be appreciated not definitely not required.
So purely for terminology consistency (since this is only for the grid codebase and we may as well use the same terms) I would opt for
const { rowId, columnName } = parseCellID(cellId)
const cellId = getCellID(rowId, columnName)
But that's just me being overly anal about naming. Functionally this looks perfect and is much more robust and maintainable. Meant to do this myself at some stage rather than have the duplicated logic all over the place. Nice one!
Description
Fixing an issue with ID/Rev not being copyable from context menu due to the way IDs were split/handled in the grid.
External rows can have row IDs which contain
-
characters, this is used in the grid store to attach the field which is currently focused, this can be avoided by assuming the last split off-
will leave the ID as the first element and the field as the second element.Addresses