Penalty Tracking Page Enhancements #112
Conversation
1. use operator names/colors instead of overlay names/colors 2. update Name/Alternate Name when it is updated elsewhere, instead of requiring a refresh or requiring skater changes to listen for it
1. make background color very dark grey 2. remove opacity setting from header, it just makes the colors look washed out and weird
re-indent after adding wrapping IIFE
| teamMock = Mockito.mock(TeamModel.class); | ||
| skaterId = UUID.randomUUID(); | ||
|
|
||
| model = new DefaultSkaterModel(teamMock, skaterId.toString(), "Test Skater","123",""); |
There was a problem hiding this comment.
Spaces after comma is inconsistent here
| @@ -151,6 +152,7 @@ public void AddPenaltyModel(String id, boolean foulout_explusion, int period, in | |||
| if (penalties.size() < 9) { | |||
| DefaultPenaltyModel dpm = new DefaultPenaltyModel(id, period, jam, code); | |||
| penalties.add(dpm); | |||
| sortPenalties(); | |||
There was a problem hiding this comment.
The EVENT_PENALTY is no longer just about that individual penalty, as it could reorder other penalties. Thus I think the event should be considered to be more generic and not pass dpm to make that clearer. This doesn't require code changes in xml/json listeners as they currently replace the entire skater.
There was a problem hiding this comment.
I agree that a single penalty object is not ideal, but I also want to avoid the potential pitfalls of not sending anything at all in as the event value (as that would violate the convention that null is a removal action of some sort). For now I've updated it to be the skater's list of penalties, but I can update it to be something else (like total count of penalties, a string/enum indicating the action type, etc) if you think that's more appropriate.
|
Here's a screenshot of the various UI updates, showing two skaters on the track, with penalties in both the prior period, a prior jam in the same period, and the current jam. I will push some fixes momentarily to address the code review issues, plus a few bugs related to reloading the penalty tracking page I discovered while making screenshots. |
|
Thanks! |
User-facing changes to the Penalty Tracking page:
Additionally, I've made the following set of backend/developer changes
This doesn't technically do what #66 asks, in that it's not an option and it's not color, but I am pretty much alright with something opinionated for now. I am not totally sure that coloring the prior periods penalties justifies adding a set of operator settings/login functionality/etc on it's own. Maybe as we build up a few more settings for PT we can re-address.
I had to make a bunch of whitespace changes to index.js to keep it formatted properly which sort of screws up the unified diff, unfortunately.
Closes #91
Closes #101