dot/LED view, w/ dismissable pulse on state change #128

sadielbartholomew · 2019-07-15T16:48:04Z

WIP (awaiting agreed cross-view data provision approach, see ⭐). Close #79 & provide a POC example for #127.

Cylc 8 UI live implementation of the old Dot/LED gcylc view, with some (proposed) changes:

actual dots (circles, not squares)!
(see Task &/or job state transition monitoring #127) a new feature where the dot icons undergo a pulse animation (like an LED!) when the task's state changes, until the user clicks to dismiss it, or refreshes the whole page to dismiss all pulsating tasks.

Tips for those (eventually) reviewing (or just trying this branch out):

You can use cylc reset to manually reset some task states to test the pulse animation.
For this branch (& PR) I've re-based to master & systemised my commits, but I developed this all on another branch, should you be confused as to timestamps implying this took just a few hours!

This PR will (with progress indicated):

Add a new URL path for each of a user's suites at .../suites/dot/<suite name> (renaming the Tree View path correspondingly to .../suites/tree/<suite name>), with both accessible, for now, via separate icons in individual table columns from the Suites Listing page (.../suites).
Incorporate text for internationalisation on the Dot & Tree View tables' headings in both currently-supported languages.
- Note: I've simply Google-translated the headings text from English to Portugese for this, @kinow please let me know how bad it did; I definitely anticipate changes will be needed to this "Portugese"!
Populate each suite's /dot/ page with a table with information for all tasks for a suite.
Recast the table, making it dynamically-sized with cycle points as columns rather than them being the top-level of the nested structure of task data.
- ⭐: I am waiting for a resolution (GraphQL Proxy #123?) to central data store #115 before attempting this, because I have struggled to get this aspect working with the current nested model for subscription queries (which also goes through a transformer adding a further layer of manipulation); a flat model for them would make this much simpler to implement, & the implementation much cleaner!
Replace the text-based report of the task state with icons of circles with appropriately-mapped colours taken from the status-indicator Vue component which has been incorporated into the repository.
Customise those icons so they are larger than the default provided by the component, to better sit with the table & page dimensions.
Apply the pulse animation: the dot icons (as above) will pulse any time a task changes state & do so until they are clicked.
Revise (after discussion) the colour mappings given to each task state, where we may want to create further colours on dots, as there are only five currently.

~~Create tests (or do so as a follow-on PR)?~~ [agreed that in context this should be future work]

Concluding self-review (ensure tests pass).

kinow · 2019-07-16T01:19:10Z

actual dots (circles, not squares)!

😄 that makes sense I guess, for the dot view.

Create tests (or do so as a follow-on PR)?

For now I think the Graph component from Martin is the only one that included tests. I think we can move the tests to a follow-on, as we are all more or less changing the whole code and a lot will change (especially regarding the central store).

I have a post-it to start investigating how to write simple unit tests for the small components (Task, Job, new Header). For views it will be a little harder I think.

kinow · 2019-07-16T01:36:39Z

src/components/cylc/Dot.vue

+
+<script>
+  // This script customises the default 'dot' icons and animations on them:
+  import 'status-indicator/styles.css'


At least for me the component was installed in node_modules as vue-status-indicator... the IDE failed to locate the CSS, but this one worked? import 'vue-status-indicator/styles.css' (I think their docs also use this value).

Ah, great spot, thank you! For context, there are actually two separate components status-indicator & vue-status-indicator (where the creator specifies on the README.md for the latter 'This is a fork of status-indicator with a few changes for use with Vue.js'), & on my development branch by mistake I installed both instead of just the Vue version. And when I tidied up for this branch, I removed the non-Vue variation as I got build errors for it without it in my package-lock.json, without realising, as it turns out from your observation, I was importing the wrong one. Then I was very confused as to why Travis threw an error in the same way on the status-indicator aspect, which I thought had gone completely from the code!

I'll get vue- prepended in my next commit.

kinow · 2019-07-16T01:38:08Z

src/components/cylc/Dot.vue

+        if (stateString == null ||
+            this.stateToDotClassMappings[stateString] == null ||
+            this.stateToDotClassMappings[stateString] != requiredState) {
+          return false;


In JS it is safer to use === and !==... https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness.

I think this function can be rewritten as:

return !(stateString == null || this.stateToDotClassMappings[stateString] == null || this.stateToDotClassMappings[stateString] != requiredState);

In JS it is safer to use === and !==

Sure, I remember reading about that. But in this case I have just checked & null === undefined evaluates to false, so I think I might need to change null -> undefined. too in that case. I'll hack the code to try & observe these cases, & see what works; I want to make sure all error cases are covered. I guess I am adjusting from my native language, Python, as there isn't a KeyError equivalent raised in JS for an invalid dict key.

I think this function can be rewritten

Absolutely, good point (confused as to how I missed that, as it is pure Boolean logic, but I didn't really do a self-review for consolidation etc. at this stage, & think I was just eager to get everything working).

I would probably write it like you did too. My IDE warned me about the === and also suggest the rewrite 😁

Good old emacs doesn't have that sort of feature, sadly. Maybe it is time for me to transition to using Atom 🤔

Actually, for your suggestion on strict (in)equality being safer than abstract (in)equality for comparison, I have just amended the final !=, because (I forgot I already checked this, hence my original approach!) in the first pair of cases, comparing in the abstract to null catches both the null & undefined, whereas comparing strictly will only catch one. There's a good thread on that here that I learnt this from.

kinow · 2019-07-16T01:48:33Z

src/components/cylc/Dot.vue

+    v-on:click="dismissPulse()"
+    :pulse="isPulseOn"
+  >
+  </status-indicator>


I think you will still need to load the component. You can do it local or globally .

Something like this

diff --git a/src/components/cylc/Dot.vue b/src/components/cylc/Dot.vue index 8f535eb..0bdb955 100644 --- a/src/components/cylc/Dot.vue +++ b/src/components/cylc/Dot.vue @@ -22,11 +22,15 @@ <script> // This script customises the default 'dot' icons and animations on them: - import 'status-indicator/styles.css' + import 'vue-status-indicator/styles.css' + import { StatusIndicator } from 'vue-status-indicator' export default { name: 'Dot', props: ['dotClass'], + components: { + 'status-indicator': StatusIndicator + }, watch: { // Start the pulse animation if the task state (hence dot class) changes hasDotClassChanged: function() {

I see, thank you for the advice. That does make full sense, though strangely it worked for me without (for reasons unknown). I'll add this in for the next commit anyway, for reassurance.

kinow · 2019-07-16T01:53:17Z

src/components/cylc/Dot.vue

+      }
+    }
+  }
+</script>


Great work on linking everything together! The computed property, props, watcher. 👏 👏 👏 ! I still get confused and sometimes have to go for a walk when things start to get too complicated with all the values and their relationships 😁

kinow · 2019-07-16T02:43:18Z

src/lang/pt-BR/DotView.json

@@ -0,0 +1,4 @@
+{
+  "tableHeader": "Ponto Vista",
+  "tableSubHeader": "O status das tarefas de um conjunto conforme organizado por ponto de ciclo"


Ponto Vista sounds like Point of View in Portuguese (ponto de vista). But "vista" doesn't really mean the same as in English like a display or visualization... I guess we would need to go with visualization here.

Maybe "Visualização por Pontos" for the tableHeader and for tableSubHeader "Os status das tarefas de uma suite organizados por pontos do ciclo."

Excellent, thank you. The subtleties of translation 😁 Google translate has a tough job, for sure!

kinow · 2019-07-16T02:44:41Z

src/lang/pt-BR/Suites.json

@@ -1,6 +1,6 @@
 {
  "tableHeader": "Lista de Suites",
-  "tableSubHeader": "Esta é a lista de suites que este usuário possui permissoões para visualizar",
+  "tableSubHeader": "Esta é a lista de suites que tenho permissoões para visualizar",


s/tenho/possuo (can't explain why... in this case the verb possuir/possess I guess sounds better)... but if you could fix my old typo as well, please haha. s/permissoões/permissões (extra o)

Oh, BTW, just to clarify in case there is the potential for misinterpretation, I was not changing your own translation here (I do not know Brazilian Portuguese of course so to change your translation would be ridiculous), it is that I changed the text slightly in so that for the en-GB it reads 'I have' rather than 'the current user', which is more appropriate I think (probably the previous was for information during development). So I tried to change the pt-BR equivalent using Google translate, awaiting some help from a native speaker 😄.

Thanks for help with the translations here, I'll add them in a new commit soon. Let me know if I missed anything once that goes up.

kinow · 2019-07-16T02:54:58Z

src/lang/pt-BR/TreeView.json

@@ -0,0 +1,4 @@
+{
+  "tableHeader": "Vista de árvore",
+  "tableSubHeader": "Dados personalizáveis baseados em colunas para todas as tarefas desta suíte"


Grammatically correct, but sounds like "The view from a tree". I think for tableHeader "Visualização em Árvore". And for the tableSubHeader maybe "Dados em formato colunar e customizáveis disponíveis para as tarefas desta suite." ("suíte" is the correct translation, but we still use "suite" as in "suite de testes" / "test suite" and "suíte" more for a hotel room name... can't explain why 😕 )

"The view from a tree" - he heh, I like that. I love being up a tree, looking out at the landscape. 😁

kinow · 2019-07-16T03:00:15Z

I really liked this status indicator! Great idea @sadielbartholomew ! Here's what it looks like after applying a few changes mentioned above, then running NODE_ENV=offline npm run serve, and visiting http://localhost:8080.

Or what it looks after merging locally the #129 changes:

And because it looks much nicer with the animation:

Bravissimo Sadie! Looking forward to being able to have Gscan, Tree view, Dot view, and Graph all working together! We are almost there

codecov-io · 2019-07-16T14:26:37Z

Codecov Report

❗ No coverage uploaded for pull request base (master@1d341ab). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #128   +/-   ##
=========================================
  Coverage          ?   80.62%           
=========================================
  Files             ?       16           
  Lines             ?      160           
  Branches          ?        0           
=========================================
  Hits              ?      129           
  Misses            ?       31           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d341ab...b5cdfe6. Read the comment docs.

sadielbartholomew · 2019-07-16T14:51:24Z

Tests now passing (after importing vue-status-indicator rather than status-indicator, as per #128 (comment)).

I have a few non-development tasks to address urgently, so won't get back onto this PR until tomorrow. But the next step is to negotiate with everyone how we plan to go ahead with the data provision so I can access the data to formulate it in the Dot View arrangement with cycle point columns.

hjoliver · 2019-07-16T19:55:53Z

Nice, great progress.

hjoliver · 2019-07-16T19:56:12Z

@kinow - how did you record that gif, BTW?

kinow · 2019-07-16T20:04:46Z

@kinow - how did you record that gif, BTW?

With byzanz. I think I used this post to install it. I can't remember if I modified anything further, but I also have the script I run here

oliver-sanders · 2019-07-18T10:53:56Z

src/components/cylc/Dot.vue

+  The second group start, upon a state (hence colour) change for that task, and
+  stop, upon a user click onto that dot, the pulse animation.
+  -->
+  <status-indicator


Should use or compose the <task> component here?

I went for different components, at least for now, since:

(Predominantly) we haven't decided how to incorporate task-job separation into this view. I don't think the approach applied for the Tree View & Graph View mock-ups, with physically-separated icons for the jobs & tasks, is a good approach here, as since state icons are the only information in this view & they are all condensed in a close-packed arrangement, it would become a cluttered mess of icons. I plan to draw up some ideas on the Design Issue for how to indicate both task & job status here.

(Also) I wanted to get a POC for state change animations, & we do not have any animated components ready yet (or indeed a decision on how they would look when animated).

The dot view is a little awkward here I agree but having separated the task an job icons we really don't want to go back to merging them. I guess would expect the dot view to just show task state with job state available on request (e.g. context menu)?

but having separated the task an job icons we really don't want to go back to merging them

Agreed, I wasn't suggesting that. We should be definitely be consistent with the segregation. I was thinking either along the line you suggest, where the table could be toggled to display either all tasks or all jobs (collecting multiple jobs for one task entry together in some condensed way, like in a self-contained grid rather than linearly in a row, for clarity), or having a new design component that indicates both task & job status in a discrete way (all within one shape). I will try to get up some drawings soon.

sadielbartholomew · 2019-08-25T21:21:06Z

Hi all, I will close this PR since the code is probably too out of date now, such that it will be easier for me to start from scratch, but I would still like to implement the Dot View eventually (I think the natural time to add it would be when @kinow's Tree Component is ready), where I will use the experience in Vue & the cylc-ui codebase I gained here to guide me in the new attempt.

We can also view this closed PR as a POC for task state transition animations.

sadielbartholomew added 5 commits July 15, 2019 13:41

Include status-indicator module and CSS to customise defaults

a035822

Create Dot Vue component which adapts with task state & history

99f17f5

Create blank Dot View pages accessible via Suites List + apply i18n

2f7b743

Add table w/ Dot components to Dot View page (needs CPs as columns)

7da3364

Apply one i18n tweak missed out in commit 2f7b743

bdb5d62

sadielbartholomew added the enhancement label Jul 15, 2019

sadielbartholomew requested review from kinow and matthewrmshin July 15, 2019 16:48

sadielbartholomew self-assigned this Jul 15, 2019

sadielbartholomew changed the title ~~dot/LED view, including dismissable pulse on state change~~ dot/LED view, w/ dismissable pulse on state change Jul 15, 2019

kinow reviewed Jul 16, 2019

View reviewed changes

sadielbartholomew mentioned this pull request Jul 16, 2019

Task &/or job state transition monitoring #127

Open

Respond to feedback + learn some (Brazil) Portuegese

b5cdfe6

oliver-sanders reviewed Jul 18, 2019

View reviewed changes

sadielbartholomew closed this Aug 25, 2019

kinow added this to the 0.1 milestone Sep 10, 2019

kinow mentioned this pull request Oct 30, 2019

Dot (LED) View #79

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dot/LED view, w/ dismissable pulse on state change #128

dot/LED view, w/ dismissable pulse on state change #128

sadielbartholomew commented Jul 15, 2019 •

edited

Loading

kinow commented Jul 16, 2019

kinow Jul 16, 2019

sadielbartholomew Jul 16, 2019

kinow Jul 16, 2019

sadielbartholomew Jul 16, 2019 •

edited

Loading

kinow Jul 16, 2019

sadielbartholomew Jul 16, 2019 •

edited

Loading

sadielbartholomew Jul 16, 2019 •

edited

Loading

kinow Jul 16, 2019

sadielbartholomew Jul 16, 2019 •

edited

Loading

kinow Jul 16, 2019

kinow Jul 16, 2019

sadielbartholomew Jul 16, 2019

kinow Jul 16, 2019

sadielbartholomew Jul 16, 2019

kinow Jul 16, 2019

hjoliver Jul 16, 2019

kinow commented Jul 16, 2019 •

edited

Loading

codecov-io commented Jul 16, 2019 •

edited

Loading

sadielbartholomew commented Jul 16, 2019

hjoliver commented Jul 16, 2019

hjoliver commented Jul 16, 2019

kinow commented Jul 16, 2019

oliver-sanders Jul 18, 2019 •

edited

Loading

sadielbartholomew Jul 18, 2019

oliver-sanders Jul 18, 2019

sadielbartholomew Jul 18, 2019

sadielbartholomew commented Aug 25, 2019

dot/LED view, w/ dismissable pulse on state change #128

dot/LED view, w/ dismissable pulse on state change #128

Conversation

sadielbartholomew commented Jul 15, 2019 • edited Loading

Tips for those (eventually) reviewing (or just trying this branch out):

This PR will (with progress indicated):

kinow commented Jul 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sadielbartholomew Jul 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sadielbartholomew Jul 16, 2019 • edited Loading

Choose a reason for hiding this comment

sadielbartholomew Jul 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sadielbartholomew Jul 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kinow commented Jul 16, 2019 • edited Loading

codecov-io commented Jul 16, 2019 • edited Loading

Codecov Report

sadielbartholomew commented Jul 16, 2019

hjoliver commented Jul 16, 2019

hjoliver commented Jul 16, 2019

kinow commented Jul 16, 2019

oliver-sanders Jul 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sadielbartholomew commented Aug 25, 2019

sadielbartholomew commented Jul 15, 2019 •

edited

Loading

sadielbartholomew Jul 16, 2019 •

edited

Loading

sadielbartholomew Jul 16, 2019 •

edited

Loading

sadielbartholomew Jul 16, 2019 •

edited

Loading

sadielbartholomew Jul 16, 2019 •

edited

Loading

kinow commented Jul 16, 2019 •

edited

Loading

codecov-io commented Jul 16, 2019 •

edited

Loading

oliver-sanders Jul 18, 2019 •

edited

Loading