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

feat: Route Set UI 2 #344

Merged
merged 7 commits into from Oct 7, 2020
Merged

feat: Route Set UI 2 #344

merged 7 commits into from Oct 7, 2020

Conversation

jstarpl
Copy link
Member

@jstarpl jstarpl commented Oct 1, 2020

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

This PR introduces a new UI for the Route Sets.

  • What is the current behavior? (You can also link to an open issue here)

The UI for the Route Sets consists of a couple of buttons in the sidebar in Rundown View and a modal confirmation box for enabling/disabling.

  • What is the new behavior (if this is a feature change)?

All of the controls are combined into a panel, next to the support panel, with buttons to toggle the routes on and off.

  • Other information:

The Studio has a new map that allows specifying properties for the exclusivity groups. Currently, it's just user-facing labels.

Status

  • The functionality has been tested by the PR author
  • The functionality has been tested by NRK

@jstarpl jstarpl requested a review from nytamin October 1, 2020 12:57
@nytamin
Copy link
Contributor

nytamin commented Oct 2, 2020

When there are exactly 2 RouteSets in an exclusivity group, they should turn into a single selector:
image

vs

image

EDIT: Disregard this comment, it works correctly. I had forgotten to set the Behaviour of the routeSet to ACTIVATE_ONLY.

@nytamin
Copy link
Contributor

nytamin commented Oct 2, 2020

Another note:
Could you flip the toggles, so that "on" is to the right, instead of left?
(I asked Jonas to flip the design sketches as well)

image

Copy link
Contributor

@nytamin nytamin left a comment

Choose a reason for hiding this comment

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

I've read through the code and code-wise it looks all good!

I put some remarks on the GUI functionality as separate comments though.

@jstarpl
Copy link
Member Author

jstarpl commented Oct 2, 2020

When there are exactly 2 RouteSets in an exclusivity group, they should turn into a single selector:
image
image

The 2 RouteSets in a single exclusivity group combined into one kicks in if both of those route-sets are set as "activate-only".

@jstarpl
Copy link
Member Author

jstarpl commented Oct 2, 2020

Another note:
Could you flip the toggles, so that "on" is to the right, instead of left?
(I asked Jonas to flip the design sketches as well)

image

I've flipped the labels.

@jstarpl jstarpl requested a review from nytamin October 2, 2020 14:00
@codecov-commenter
Copy link

Codecov Report

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

Impacted file tree graph

@@             Coverage Diff              @@
##             release26     #344   +/-   ##
============================================
  Coverage             ?   67.21%           
============================================
  Files                ?      189           
  Lines                ?    12774           
  Branches             ?     2843           
============================================
  Hits                 ?     8586           
  Misses               ?     4121           
  Partials             ?       67           

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 90d303d...bd65d4f. Read the comment docs.

Copy link
Contributor

@nytamin nytamin left a comment

Choose a reason for hiding this comment

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

Looks good!

@jstarpl jstarpl merged commit f8058a0 into release26 Oct 7, 2020
@jstarpl jstarpl deleted the feat/routeSetUi2 branch October 7, 2020 09:47
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.

None yet

3 participants