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

Unexpected results when reordering with hidden columns #152

Open
vitch opened this issue Apr 19, 2023 · 3 comments
Open

Unexpected results when reordering with hidden columns #152

vitch opened this issue Apr 19, 2023 · 3 comments

Comments

@vitch
Copy link
Contributor

vitch commented Apr 19, 2023

As seen here:

hidden-columns

When "Column A" is re-shown I would expect it to still be at the start of the list.

From initial digging it seems like the reordering only happens on the visible columns and any non-visible columns are just appended to the end of the list. I think we need to keep invisible columns in the data structure that we are reordering

@NullVoxPopuli
Copy link
Contributor

oh no! It's certainly intended to behave the way you expect: https://github.com/CrowdStrike/ember-headless-table/blob/main/ember-headless-table/src/plugins/column-reordering/plugin.ts#L215

If you have time, there is an existing test you could use to make a reproduction of the behavior you've found: https://github.com/CrowdStrike/ember-headless-table/blob/main/test-app/tests/plugins/column-reordering/rendering-test.gts#L337 <3

@vitch
Copy link
Contributor Author

vitch commented May 16, 2023

Planning to take a look at this tomorrow...

From an initial poke around it seems like we'll need to remove the coupling between the column-visibility and column-reordering plugins. Currently the reordering plugin receives the columns with visibility already applied so it won't be able to sensibly apply sorts. I think that the reordering plugin will have to run first - does that seem right?

@NullVoxPopuli
Copy link
Contributor

For this behavior/ requirement, the order can't be changed, i don't think: https://github.com/CrowdStrike/ember-headless-table/blob/main/ember-headless-table/src/plugins/column-reordering/plugin.ts#L217

This list, should probably maintain all columns (there's an api for that!)

https://github.com/CrowdStrike/ember-headless-table/blob/main/ember-headless-table/src/plugins/column-reordering/plugin.ts#L200

And then 'orderedColumns' can only return the visible columns.

This works well with the swap-only approach to reordering

@vitch vitch mentioned this issue May 17, 2023
12 tasks
vitch added a commit that referenced this issue May 17, 2023
As described in #152, the behaviour when reordering over hidden columns
is incorrect. This test illustrates the expected behaviour so that we
can fix the implementation.
vitch added a commit that referenced this issue May 17, 2023
As described in #152, the behaviour when reordering over hidden columns
is incorrect. This test illustrates the expected behaviour so that we
can fix the implementation.
vitch added a commit that referenced this issue May 24, 2023
As described in #152, the behaviour when reordering over hidden columns
is incorrect. This test illustrates the expected behaviour so that we
can fix the implementation.
vitch added a commit that referenced this issue Jul 4, 2023
As described in #152, the behaviour when reordering over hidden columns
is incorrect. This test illustrates the expected behaviour so that we
can fix the implementation.
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

No branches or pull requests

2 participants