Unit overview bugs #14509
Merged
Unit overview bugs #14509
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bjvanminnen
force-pushed
the
unitOverviewBugs
branch
from
2e911ad
to
06ccd11
Compare
Bjvanminnen
force-pushed
the
unitOverviewBugs
branch
from
06ccd11
to
e545d99
Compare
Bjvanminnen
force-pushed
the
unitOverviewBugs
branch
from
09b45f3
to
e09cee5
Compare
Bjvanminnen
force-pushed
the
unitOverviewBugs
branch
from
e09cee5
to
9b18285
Compare
|
Sorry for the delay on this, I'm looking now 👀 |
islemaster
reviewed
Apr 20, 2017
| @@ -139,6 +141,13 @@ export default function reducer(state = initialState, action) { | |||
| }; | |||
| } | |||
|
|
|||
| if (action.type === SET_CURRENT_STAGE_ID) { | |||
| return { | |||
| ...state, | |||
islemaster
approved these changes
Apr 20, 2017
| }, | ||
| ], | ||
| fakeLevels(2), | ||
| fakeLevels(2), | ||
| fakeLevels(2) | ||
| ]; | ||
|
|
||
| console.log(levelsByLesson); |
There was a problem hiding this comment.
I'm guessing you didn't mean to leave this console.log call in.
| }; | ||
| }, | ||
|
|
||
| componentWillReceiveProps(nextProps) { | ||
| if (nextProps.currentStageId !== this.props.currentStageId) { | ||
| this.setState({ |
There was a problem hiding this comment.
Interesting... coverage suggests we test this behavior but not its negative case: A props change that does not change the current stage should not change the collapsed state. Worth covering?
| }); | ||
|
|
||
| it('renders a pill that says UNPLUGGED when first level is unplugged', () => { | ||
| it('renders a pill with no text when first level is unplugged', () => { |
There was a problem hiding this comment.
Should there be a matching test "renders a pill with no text when last level is unplugged?"
| @@ -744,6 +744,17 @@ def summarize | |||
| summary | |||
| end | |||
|
|
|||
| # Similar to summarize, but returns an even more narrow set of fields, in particular | |||
| # bypasses stage summaries | |||
| def summarize_short | |||
There was a problem hiding this comment.
I know this gets hit in tests, but it'd be nice to have a new unit test pinning just the behavior of summarize_short, since it's new public API for this model.
This was referenced Apr 21, 2017
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A few fixes for small unit overview bugs
Miniview should start in detail mode when that is the script default
No LEVEL text in pills on left
Teacher Info extends full heigh on FF/Safari/IE
Default all stages but the current one to be collapse in default view (both on overview page and in miniview)
