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

Templating Task Name #863

Merged
merged 21 commits into from
Jun 9, 2021
Merged

Templating Task Name #863

merged 21 commits into from
Jun 9, 2021

Conversation

ThatGalNatalie
Copy link
Contributor

@ThatGalNatalie ThatGalNatalie commented May 27, 2021

PR Checklist:

  • add a short description of what's changed to the top of the CHANGELOG.md
  • add/update tests (or don't, for reasons explained below)

Describe this PR

Should resolve #853

@znicholasbrown
Copy link
Contributor

I think we can show both the task run name and the task name, something like this for flow runs:

<<task name>> (test-subtitle-2 font-weight-light)
<<task run name>> (text-h5)
<<duration>> (text-subtitle-2)

but then for mapped tasks we could do:

<<task name>> (text-h5)
<<number of finished runs / total number>> (text-subtitle-2 font-weight-light)
<<duration>> (text-subtitle-2)

(note that those are relative font classes, I didn't check what the current ones are or whether they're even class controlled)

We'd want to keep the static graph as-is.

@znicholasbrown
Copy link
Contributor

It looks like we're still showing special class type icons on the task name line. This view might be confusing with duplicates when runs aren't explicitly named, but perhaps not. Maybe making bumping the overline text down a little would help?
Screen Shot 2021-06-01 at 12 11 41 PM

@znicholasbrown
Copy link
Contributor

I think there might be some unpushed changes, this looks largely the same as before:
Screen Shot 2021-06-04 at 2 42 08 PM
Note that the special class type icons are still duplicated on the task name line.

On the finished runs portion, that can be said more succinctly like this:

current/total runs complete

and when zoomed out far enough that the chart/duration are no longer showed, probably just current/total.

I think this has introduced a small bug with node scaling/placement (note that terminal nodes have shifted down somewhat and are no longer centered on the end of the edge):
this branch:
Screen Shot 2021-06-04 at 2 44 30 PM
master:
Screen Shot 2021-06-04 at 2 44 34 PM

Copy link
Contributor

@znicholasbrown znicholasbrown left a comment

Choose a reason for hiding this comment

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

This looks good, a few small code comments

src/components/Schematics/Schematic-Node.vue Outdated Show resolved Hide resolved
src/components/Schematics/Schematic-Node.vue Outdated Show resolved Hide resolved
@ThatGalNatalie ThatGalNatalie dismissed znicholasbrown’s stale review June 8, 2021 20:26

Added in the new changes

Copy link
Contributor

@znicholasbrown znicholasbrown left a comment

Choose a reason for hiding this comment

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

Nice, thanks for the updates @ThatGalNatalie 👍🏻

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.

BUG: Templating Task Names Updates Task List in UI but not Schematic
2 participants