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

persist selected row #1135

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nthangeniphumudzo
Copy link
Contributor

Hi @IvanIlyichev . I was tasked to do this item. My changes are merely just making sure that we move away from the entire 'document' and just focus on the "table body" to avoid deSelect when clicking outside as per item stipulated.

Copy link
Contributor

@IvanIlyichev IvanIlyichev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nthangeniphumudzo .This change is incorrect and doesn't fix the issue you mention.
I think the initial logic was to clear the selection when a user clicks outside the table, but in some cases it makes sense to keep the selection.
The change you proposed just moves the event handler and to a first table in the DOM.
If I understand correct this handler should be removed entirely, but if it's still required it should be attached to a correct DOM element, not to a first table, it will work incorrect if your page has multiple tables.

@nthangeniphumudzo
Copy link
Contributor Author

Thanks @IvanIlyichev for the insightful feedback. I will revise my implementation and communicate when I am done.

@nthangeniphumudzo
Copy link
Contributor Author

Hi @IvanIlyichev . I have made use of useRef distinguish each table and use it to demacate the inside/outside of each table. Please kindly review and feedback . Thanks

Copy link
Contributor

@IvanIlyichev IvanIlyichev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nthangeniphumudzo. Row selection is handled wrong, I'll review it myself a bit later

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.

None yet

2 participants