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

Prevent row expansion if row is already expanded #626

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

Conversation

nb1987
Copy link

@nb1987 nb1987 commented Mar 22, 2017

I encountered an odd issue when using an x-editable <select> in FooTable -- when updating the value of the <select> in a cell on an expanded FooTable row, expand() is invoked despite the row already being in an expanded state. Thus, the cells in the expanded row end up being collapse()d, resulting in the clearing of their contents. I spent a bit of time puzzling over this but ultimately resolved it simply by adding a sanity check at the start of expand. I assume this check would be prudent anyway.

@steveush
Copy link
Member

Hi @nb1987,

I'll test this out and see if it has any issues with the FooTable.Row#val() and FooTable.Cell#val() methods and if not I'll merge it in. Since you did the pull request against the develop I assume you also got the issue with x-editable using the develop branch version?

The reason I need to check the above methods is because the FooTable.Row#collapse() method does not always set the FooTable.Row#expanded property to false. When editing a cell of a row that is collapsed, FooTable collapses the row without setting the property, then on the next draw operation it is expanded again. This was done to ensure the correct cells are updated, to the user they never see the collapse happening as it is all done basically at the same time. So I may need to do some other work to incorporate your fix.

Thanks

@nb1987
Copy link
Author

nb1987 commented Mar 23, 2017

Hi @steveush,

That is correct - the issue occurred using the develop branch version. I understand about the additional checks and figured there may be a quirk or two which may necessitate said checks.

Thanks for all the work you put into FooTable - it's easily one of my favorite JS libraries!

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.

2 participants