seqeralabs / nf-tower

Nextflow Tower system
https://tower.nf
Mozilla Public License 2.0
146 stars 52 forks source link

Improve task table #90

Closed pditommaso closed 5 years ago

pditommaso commented 5 years ago
Screenshot 2019-08-09 at 09 34 14

The tasks table needs to be improved to be really useful:

I'm attaching the execution report create by NFD which can help to take inspiration and code/css snippets.

report.html.zip

tcrespog commented 5 years ago

Hi Paolo. What do you mean by "better grid layout". Right now the design is the one integrated with Bootstrap 4: https://datatables.net/examples/styling/bootstrap4.

pditommaso commented 5 years ago

The one in the table in the project home (that I guess is the default one) it's a bit better.

tcrespog commented 5 years ago

Hi Paolo, sorry for bothering you on your vacation. Feel free to answer whenever you want, these aren't blocking matters. Only two doubts:

About:

when clicked the row should expand and show (selected details) in a readable above all the content of the script column which can be big. Eventually, it could be shown a "plus" icon as in the datatable page https://datatables.net/

I've already implemented the plus icon as shown in the DataTable default demo, but do you prefer the icon or do you prefer to expand/collapse the row just by clicking the row without icon involved?

About:

sorting: is going to be a client or server-side sorting (deserve a separate issue)

We should define this behaviour.

pditommaso commented 5 years ago

I've already implemented the plus icon as shown in the DataTable default demo

Ok, let's keep the plus icon for now. @evanfloden we need to mockup a pane showing the most important task info eg. name, tag, executed script, work directory, the error message (when failing)

We should define this behaviour

Since there could be many rows, the sort should be done server-side. But then we need also to define which columns should be indexed.

I would say fow now only the following cols

    String hash
    String name
    String process
    String tag

    TaskStatus status

    OffsetDateTime submit
    OffsetDateTime start
    OffsetDateTime complete
tcrespog commented 5 years ago

Since there could be many rows, the sort should be done server-side. But then we need also to define which columns should be indexed. I would say fow now only the following cols

Ok, so do you mean sorting by those columns trigger a server-side ordering and the rest trigger a client-side ordering?

pditommaso commented 5 years ago

Nope, the rest should not be sortable

tcrespog commented 5 years ago

Ok, right now the ordering is server-side too (it's integrated along with the filtering and pagination). It should be a matter of defining just those columns as sortable.

pditommaso commented 5 years ago

Great work! A few comments here:

Question: does the datatable component support column re-ordering?

pditommaso commented 5 years ago

I've reorganised a bit the table removing the large cols eg script because they are supposed to the showed in the expandable area 49c156b.

However, I've noticed I've broken the expandable table because it was getting the data from the row. Any idea how to handle this?

tcrespog commented 5 years ago

Thank you Paolo. My answer:

memory/storage value should be round up to 1 decimal ie. 10.1 MB, zero should be 0 not 0.0.

I guess 10.0 MB should be 10also. Is that right?

Question KB/MB/GB etc are multiples of 1000 or 1024 (it should be 1024).

Isn't it working correctly? Theoretically I implemented it to use 1024.

exit: the values 2147483647 should be reported as empty

Do you mean that just the value 2147483647 should be "translated" to the string empty?

Question: does the datatable component support column re-ordering?

Yes it does, through a plugin.

I've broken the expandable table because it was getting the data from the row. Any idea how to handle this?

I will try hidding the columns.

pditommaso commented 5 years ago

I guess 10.0 MB should be 10also. Is that right?

Nope, memory/storage should have one decimal eg 10.0 MB except when zero that should be 0. If you are using a lib that makes the zero exception complicated, we can live with 0.0 for now.

Isn't it working correctly? Theoretically I implemented it to use 1024.

Then it's fine. I was just asking because some people interpret KB/MB/GB as multiple of 1000 other 1024.

Do you mean that just the value 2147483647 should be "translated" to the string empty?

Yes, because that's the Java MAX_INT that's used as a special error code. See here

Yes it does, through a plugin.

OK. good to know. Not a priority. Maybe we'll add it in the future

I will try hidding the columns.

OK, thanks

tcrespog commented 5 years ago

All done (except for the columns reordering). fecc145a9b2381cbda226094d48d704c7fdc3e7b

pditommaso commented 5 years ago

Nearly. For empty I meant an empty string, not the string empty πŸ˜„

I've fixed that, but in the duration and realtime columns I still see numbers such 0.612 s, it should be only one decimal digit ie. 0.6 s.

tcrespog commented 5 years ago

πŸ˜„ I thought about making sure about that, but I ended up betting on the wrong horse. Ok, I forgot about the time, I'll fix it.

pditommaso commented 5 years ago

Great job, this can be closed.