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

drag plugin does not factor RTL layout #129

Closed
shlomiassaf opened this issue Nov 1, 2020 · 4 comments · Fixed by #141
Closed

drag plugin does not factor RTL layout #129

shlomiassaf opened this issue Nov 1, 2020 · 4 comments · Fixed by #141
Labels
comp: core @pebula/ngrid (core package only, excluding secondary packages) comp: core-plugins @pebula/ngrid/* (All things related to core plugins the come as secondary package in @pebula/ngrid) type: bug/fix
Milestone

Comments

@shlomiassaf
Copy link
Owner

shlomiassaf commented Nov 1, 2020

What is the expected behavior?

Now with the new RTL support the drag & drop plugin should factor in the RTL direction

For example, when using column resize it will not follow the right resize direction with RTL on...

Or, when dragging it will swap the columns in the wrong order.
In RTL mode it will assume the last column (first from left) is the first, as if it's LTR mode.

What is the current behavior?

Factor in RTL layout when in RTL mode.

What are the steps to reproduce?

Go to https://shlomiassaf.github.io/ngrid/
Toggle RTL to be ON
Go to demo Infinite Scroll Performance

With the mouse drag the accountType column and swap it with the accountId column next to it.

You will see the account column appearing between column of month 5 and month 6.

This is because accountType was at column index 6 and column index 6 is also between the columns month5 & month 6 if you count from the LEFT, but we're on RTL so we should count from the right

@shlomiassaf
Copy link
Owner Author

@ronnetzer would you like to take this one? if not i'll do it no problem.

@shlomiassaf shlomiassaf added comp: core @pebula/ngrid (core package only, excluding secondary packages) comp: core-plugins @pebula/ngrid/* (All things related to core plugins the come as secondary package in @pebula/ngrid) labels Nov 1, 2020
@ronnetzer
Copy link
Contributor

ronnetzer commented Nov 2, 2020

@shlomiassaf The problem starts with Directionality, the 'value' is being set only once, in instantiation.
Instead of using directly the value, I can listen to changes (Drectionality also exposes an EventEmitter) and it will probably solve the issue but material components (like overlay and drag@drop) internally read the value instead of using the emitter so the problem will still happen in them (the row drag placeholder will be in the wrong direction for example)

In my project we're using a library we've created (@e-square/bdir) that exposes a service that extends Directionality with updating the value on dir changes and we provide this service instead of Directionality, I can do here the same but I feel like this should be fixed in Material.

Let me know what you think and in any case you can assign those RTL issues to me.

Seems like there's an open issue on this

@shlomiassaf
Copy link
Owner Author

Maybe i'm missing something.
Directionality has a change stream which does not change for top-level attribute values but if you use dir via the directive it does change.

What angular/components#5761 is talking about is supporting changes in the attribute, you're talking about supporting changes in the components themselfs?

Anyway, nGrid is not just for material, it happen to be that I only wrote one plugin extension for material and not bootstrap or something native...

@shlomiassaf
Copy link
Owner Author

Also, checked again, it's not related to the Directionality, dragging a column and releasing is not working as it should.

Also, copy paste plugin is not working as RTL copy paste should.

What I suggest is to create a stream + value in the extApi that wraps directionality so the grid has it's own type.
With this all plugins can choose if they want to support RTL.

@shlomiassaf shlomiassaf added this to the 3.0.0 milestone Nov 10, 2020
@shlomiassaf shlomiassaf added the V3 label Nov 10, 2020
@ronnetzer ronnetzer mentioned this issue Nov 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: core @pebula/ngrid (core package only, excluding secondary packages) comp: core-plugins @pebula/ngrid/* (All things related to core plugins the come as secondary package in @pebula/ngrid) type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants