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

Tasks page improvements #1017

Merged
merged 15 commits into from
May 6, 2016
Merged

Tasks page improvements #1017

merged 15 commits into from
May 6, 2016

Conversation

Calvinp
Copy link
Contributor

@Calvinp Calvinp commented Apr 29, 2016

Implements several improvements to the tasks table page:

  • Name field is changed to Task ID and Started is changed to Started At
  • Host and rack fields link to a page of all tasks on said host or rack. It is now possible to filter by rackId.
  • On screens that aren't extra small or large, the racks and memory columns now line up properly.
  • The protip is now a tooltip on both this page and the requests page
  • Tooltips on task state filter links now explain the meaning of those states. Please review the wording of these tooltips
  • Fix an error in the decommissioning tasks table that caused it to never render.

</li>
<li {{#ifEqual tasksFilter "scheduled"}}class="active"{{/ifEqual}}>
<a href="{{appRoot}}/tasks/scheduled">Scheduled</a>
<a href="{{appRoot}}/tasks/scheduled" class='has-tooltip' title="Tasks that will run when the time comes" data-toggle='tooltip'>Scheduled</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe more like 'Tasks that will run at the next scheduled interval'

@@ -40,6 +40,7 @@ Handlebars.registerHelper 'ifTaskInList', (list, task, options) ->
return options.inverse @

Handlebars.registerHelper 'ifInSubFilter', (needle, haystack, options) ->
#return options.inverse @ unless haystack
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there a reason this comment was added?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Artifact of one way I considered to fix a problem I was having. I'll remove it.

</th>
<th class="hidden-xs" data-sort-attribute="host">
Host
</th>
<th class="hidden-xs" data-sort-attribute="taskId.rackId">
<th class="visible-lg" data-sort-attribute="taskId.rackId">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reasoning for making this visible vs not and lg vs xs? I would argue that just seeing the task id, cpu and mem on a small device is the more important. And probably host over rack if we were to choose one more to add

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I just chose the one I saw first as the correct one. There probably won't be room for hosts on a smaller screen if we want to show disk resource too though.

Copy link
Member

@ssalinas ssalinas May 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have the bandwidth it'd be worth a quick pass over the bootstrap classes for the table columns to make sure they collapse nicely (e.g. are xs and lg good breakpoints or is sm better? What are the most important to display and what can we hide?). If I remember correctly, visible-lg is actually deprecated as of 3.2 in favor of visible-(size)-(display style)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a look at this now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the smallest window sizes, the task ID and started at timestamp start running off the screen. But that happens regardless of what we're showing. hidden-xs seems to be ok - everything is able to display pretty much the same at the smaller sizes down to the xs cutoff. It might not be ok for a table with more columns though. Also, the actions icons (kill task and JSON) can fit without causing a change in the way the rest of the table looks even at the smallest sizes.

Copy link
Contributor Author

@Calvinp Calvinp May 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the actions icons (kill task and JSON) can fit without causing a change in the way the rest of the table looks even at the smallest sizes.

This does not appear to be the case, however, on the other tasks tables. Only on active. So I'm going to remove hidden-xs for the actions icons for active tasks only.

@ssalinas
Copy link
Member

ssalinas commented May 4, 2016

One more comment on this. When in a filter that isn't Active, it doesn't seem like the service/worker/etc filters are working properly. For example, filters out all pending tasks on the scheduled filter unless all the request type filters are active

@ssalinas ssalinas modified the milestone: 0.6.0 May 4, 2016
@Calvinp
Copy link
Contributor Author

Calvinp commented May 4, 2016

As it turns out, pending tasks don't store their request type - you'd have to fetch the task to acquire that, which is certainly not reasonable to do for every single task.
So instead of fixing that I removed the request type filters entirely for tasks that aren't active.

@ssalinas
Copy link
Member

ssalinas commented May 6, 2016

Looks good @Calvinp , the new toggles are much nicer/easier

@ssalinas ssalinas merged commit 70c746d into master May 6, 2016
@ssalinas ssalinas deleted the task-page-improvements branch May 6, 2016 16:58
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

Successfully merging this pull request may close these issues.

2 participants