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

Action menu for table columns #981

Closed
simonw opened this issue Sep 30, 2020 · 16 comments
Closed

Action menu for table columns #981

simonw opened this issue Sep 30, 2020 · 16 comments

Comments

@simonw
Copy link
Owner

simonw commented Sep 30, 2020

At the very least I'd like a menu on each table column that lets me select sort-asc v.s. sort-desc without having to click twice.

I'd also like to be able to indicate that a column should be used for faceting (possibly only for columns that are not floating point and do not have a unique index on them).

This needs to be built with accessibility in mind - I don't want screenreaders to read out the contents of a menu as the "th" label for any given cell.

Related: #690

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

Prototype:

<style>
body {
font-family: helvetica;
}
.dropdown-menu {
  display: inline-flex;
  border: 1px solid #ccc;
  border-radius: 4px;
  line-height: 1.4;
  font-size: 12px;
  box-shadow: 2px 2px 2px #aaa;
}
.dropdown-menu ul,
.dropdown-menu li {
  list-style-type: none;
  margin: 0;
  padding: 0;
}
.dropdown-menu li {
  border-bottom: 1px solid #ccc;
}
.dropdown-menu li:last-child {
  border: none;
}
.dropdown-menu a:link,
.dropdown-menu a:visited,
.dropdown-menu a:hover,
.dropdown-menu a:focus
.dropdown-menu a:active {
  text-decoration: none;
  display: block;
  padding: 4px 8px 2px 8px;
  color: #222;
  background-color: #fff;
}
.dropdown-menu a:hover {
  background-color: #eee;
}
.hook {
  display: block;
  position: absolute;
  top: 3px;
  left: 12px;
  width: 0; 
  height: 0; 
  border-left: 5px solid transparent;
  border-right: 5px solid transparent;
  border-bottom: 5px solid #ccc;
}
</style>
<div class="dropdown-menu">
<div class="hook"></div>
<ul>
  <li><a href="#">Sort descending</a></li>
  <li><a href="#">Sort ascending</a></li>
  <li><a href="#">Facet by this</a></li>
  <li><a href="#">Count by this</a></li>
</ul>
</div>

Real-time_HTML_Editor

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

More options:

<ul>
  <li><a href="#">Sort descending</a></li>
  <li><a href="#">Sort ascending</a></li>
  <li><a href="#">Facet by this</a></li>
  <li><a href="#">Unique values</a></li>
  <li><a href="#">Count unique values</a></li>
</ul>

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

Future version could have expanding out nested side menus that let you do things like "calculate sum/avg for this column against this-other-column".

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

I think the accessible way to do this is with absolute positioning - have a menu icon in the <th> which, when clicked, causes the dropdown menu to appear as an absolutely positioned <div> that is not located within the DOM hierarchy of the<th> itself but is positioned to show up in the correct place.

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

Maybe use this as the icon:

<svg width="100px" height="100px" viewBox="0 0 255 255">
  <polygon points="0,64 128,191 255,64"/>
</svg>

Real-time_HTML_Editor

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

Another potential action:

  • Show rows where this is not blank (equivalent to is not blank filter)

This could be displayed conditionally based on if the column is detected to have any blank rows in it?

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

It would be neat to provide a JavaScript plugin hook that plugins can use to add their own options to this menu. No idea what that would look like though.

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

I think I've got everything I need to implement this now.

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

Showing "facet by this" on the primary key column doesn't make sense.

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

The arrow icon didn't make sense because I already have a triangle icon showing sort order. I'm trying a cog icon instead:

data__dogs__10_rows

simonw added a commit that referenced this issue Sep 30, 2020
@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

A bunch of things to fix:

  • It clobbers existing querystring parameters - it needs to leave these alone (but replace the current sort order)
  • Facet option should not show up if you are already faceting by that column
  • There's no way to close the menu once it has opened!
  • Accessibility: SVG icon doesn't even have an alt attribute yet. Should use ARIA when the thing appears.

It's also not visible on mobile, need to think about how that will work.

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

simonw added a commit that referenced this issue Sep 30, 2020
Also show ascending option before descending option
simonw added a commit that referenced this issue Sep 30, 2020
* Menu links now take into account existing querystring
* No longer shows facet option for primary key columns
* Conditionally displays sort/sort-desc if already sorted
* Does not show facet option if already faceted by this
@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

I could provide the "Show non-blank values" option only for columns where there are blank values visible on the current page.

This could be a bit confusing though, since the absence of that option could suggest that there are no blank values at all when that's actually not true.

One option: run a separate fetch() call that figures out if any of the columns contain blank values, which gets a bit of extra time to execute. Only show the "Show non-blank values" option in the menu once that has returned.

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

I'm going to go based just on the visible values on the current page. I think that's good enough, and it avoids the complexity involved in doing a server-side check for blank values.

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

Since this menu doesn't provide new functionality, I'm going to ignore the fact that it doesn't exist on portrait mobile view for the moment. Likewise, I'm going to skip making it accessible for the moment since lacking accessibility doesn't prevent functionality from being accessed - the menu-less experience currently works the same as the portrait mobile experience.

@simonw
Copy link
Owner Author

simonw commented Sep 30, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant