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

Sortable icon right aligned #139

Closed
DocCaliban opened this issue Jul 9, 2019 · 11 comments · Fixed by #144
Closed

Sortable icon right aligned #139

DocCaliban opened this issue Jul 9, 2019 · 11 comments · Fixed by #144
Labels
enhancement New feature or request

Comments

@DocCaliban
Copy link

DocCaliban commented Jul 9, 2019

Is your feature request related to a problem? Please describe.
The column name moves position when sorting, with current design standards, you should avoid
have elements change their location on the page, even if it's just a few pixels

Describe the solution you'd like
I think moving the sort icon to be right aligned would fix this
Example: "Movie Title ^"

Describe alternatives you've considered
I thought about just having the icon always visible, but perhaps grayed out when not the active sort., but that might be a bit too busy looking.

I don't have time today, but might be able to take a stab at this tomorrow and create a pull request

@jbetancur
Copy link
Owner

I think I was trying to stay true to material design where the icon is on the left...

https://material.io/design/components/data-tables.html#behavior

Perhaps, rdt could have a new prop that places the sort icon on the right.

I like the way mui handles sort icons where they are all to the right except the last one
https://material-ui.com/components/tables/

@DocCaliban
Copy link
Author

Thanks for the speedy reply.

Another option would be to have it always rendered, but set the opacity to 0 when it's not sorted.
I'm not firmly attached to any specific solution, just hate seeing my UI bounce. If you would like I can try a few different ideas any let you review them. Or if you prefer you can implement it.

@jbetancur
Copy link
Owner

jbetancur commented Jul 9, 2019

Np, appreciate the help. Go for it! I like the idea of setting the opacity to 0. super subtle fade in/out on/off focus as well as hover ftw 😉

e.g. https://material-ui.com/components/tables/#EnhancedTable.js

@jbetancur jbetancur added the enhancement New feature or request label Jul 9, 2019
@wiland
Copy link
Contributor

wiland commented Jul 11, 2019

Hi guys, I was looking for a more explicit way to tell the user that the column is sortable, and I like the idea of hover and focus behavior (like Material UI does). Can you please tell me if you are currently working on it or if you need any help?

@DocCaliban
Copy link
Author

I created a fork and branch, and have started looking at it, but I am not familiar with styled-components so that in combination with my day job is slowing me down, so you are welcome to hop on the issue.

Im going to keep at it when I have time so I can get a beter understanding of how styled-components works.

@jbetancur
Copy link
Owner

jbetancur commented Jul 11, 2019

@wiland I agree on material ui, and ideally, this is how I think sorting icon behavior should work
https://material-ui.com/components/tables/#sorting-amp-selecting

  • Notice that right align columns have the icon left render and all other alignments have it on the right.
  • I also like the sorting transitioning and hover.

@jbetancur
Copy link
Owner

I have some time to look at this today. I'll try to get a PR up

@wiland
Copy link
Contributor

wiland commented Jul 12, 2019

@jbetancur I've been working some time with this and I already did some changes, but it's not complete yet. Can you do some review of my code? Look at this branch compare:

https://github.com/jbetancur/react-data-table-component/compare/master...Wiland:feature/sort-icon-behavior?expand=1

@jbetancur
Copy link
Owner

@wiland Thank you!!! I just saw this, but in any case, I needed to make some architectural changes to TableCol that might conflict.
I pushed a branch if you want to take a look. Once I get this merged in perhaps we could incorporate your transitions.

https://github.com/jbetancur/react-data-table-component/tree/139-properly-align-sort-icons

@jbetancur
Copy link
Owner

btw - something is not right with sorting. every time you click a field it flips. e.g. if the previous column is asc then the when you click another column it does to desc when it should stay at asc. It should (i think) maintain the same asc or desc position.

@jbetancur
Copy link
Owner

jbetancur commented Jul 12, 2019

I' ve went ahead and fixed the sorting behavior as well as some janky TableCol logic that I wrote when I first created this component.

What still remains is sort icon transitions on hover - which I will leave to a later time or if @wiland, anyone else wants to tackle.

@wiland Thanks again for the PR, but I realized I needed to make a ton of fixes to the sorting logic

Expect a 2.0.x release with these fixes soon. I had to bump the major version as this does change things quite a bit, but should not break any existing behavior (unless you are using the TableCol css hook)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants