Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Updating sortable columns to have sorting icon and updated cursor #558

Merged
merged 1 commit into from
Jul 22, 2016

Conversation

pete-the-pete
Copy link
Contributor

Fixes #328

Changes proposed in this pull request:

  • add new icon for default state of sortable-column
  • add 'sortable-column' classname to sortable-column component
  • add a new css rule to use the pointer cursor for .sortable-column class

sorting icons

@jglovier jglovier added Needs UX Feedback in progress indicates that issue/pull request is currently being worked on labels Jul 8, 2016
@jglovier
Copy link
Member

jglovier commented Jul 8, 2016

cc @HospitalRun/core-maintainers

@jglovier
Copy link
Member

jglovier commented Jul 8, 2016

@pete-the-pete thanks for getting started on this! Only design feedback I have at the moment is we should probably reduce the size of the icons a bit, and instead of the double arrows, lets use a single triangle everywhere of this style. To differentiate between active filtering and non-active, lets take a queue from existing patterns and make the triangle color lighter on the non-active filters, and darker on active filters.

@jglovier
Copy link
Member

jglovier commented Jul 8, 2016

If you'd like visual help beyond my written description above, I can post a design comp when I have some time in the next few days.

@pete-the-pete
Copy link
Contributor Author

I can take a look tomorrow and post an update.

@pete-the-pete pete-the-pete force-pushed the master branch 2 times, most recently from 429dea8 to 8accc25 Compare July 11, 2016 02:57
@pete-the-pete
Copy link
Contributor Author

I updated to use the arrows and lighter color, but I am seeing this issue, stylelint/stylelint#1513

@jkleinsc
Copy link
Member

@pete-the-pete looks good to me. @jglovier what do you think:

screen shot 2016-07-11 at 2 51 16 pm

@jglovier
Copy link
Member

The new icons looks good, except I think we should only display the active arrow, and hide the others until you hover over it's associated column heading.

@jkleinsc
Copy link
Member

@jglovier if you look the issue this PR is tied to, #328, it seems the issue is discoverability. If we don't show any icons, how will users know they can search?

@jkleinsc
Copy link
Member

@jglovier ping... what do you think? I would like to move this PR along.

@jglovier
Copy link
Member

@jkleinsc, sorry, to be more clear I think by default we should always show an icon for the currently active sorted column. So there is always one sorting icon visible.

Also, I'm not too concerned about discoverability because this is a feature most users will probably assume is there already. Sorting tabular data by column is more of an expected behavior than a novel feature we need to explicitly reveal.

@jkleinsc jkleinsc merged commit e6e6f4d into HospitalRun:master Jul 22, 2016
@jkleinsc
Copy link
Member

I am merging this PR as is. We can cleanup when the icons show as another issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
in progress indicates that issue/pull request is currently being worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants