Some small progress redesign fixes #13935
Conversation
| @@ -66,12 +67,12 @@ const ProgressBubble = React.createClass({ | |||
| href = url + location.search; | |||
| } | |||
|
|
|||
| const tooltipId = _.uniqueId(); | |||
There was a problem hiding this comment.
This was done because in storybooks we end up with multiple bubbles that have the same URL (even though I think this should never happen on a real page). I alternatively could have tried doing the work to make our storybooks always provide unique urls, but this approach seems more resilient.
| description={"At some point we reach a physical limit of how fast " + | ||
| "we can send bits and if we want to send a large amount of " + | ||
| "information faster, we have to finds ways to represent the same " + | ||
| "information with fewer bits - we must compress the data."} |
There was a problem hiding this comment.
We don't actually use description anywhere right now, so it seems silly to have a storybook for it.
| * @param {ViewType?} viewAs - Optional param to determine whether the lesson | ||
| * would be visible if viewing as someone else | ||
| * @param {ViewType} viewAs - Are we interested in whether the lesson is viewable | ||
| * for students or teachers | ||
| * @returns {boolean} True if the provided lesson is visible | ||
| */ | ||
| export function lessonIsVisible(lesson, state, viewAs) { |
There was a problem hiding this comment.
Previously we would call this method sometimes providing a viewAs, and sometimes not (in which case we would default to whatever was currently in the redux store). This was confusing, and I never really liked it. Now that SummaryProgressRow has viewAs as a prop (which will be the same as whatever is in the redux store), it's cleaner to just always pass in the viewAs prop and not have this as an optional param.
|
read through all of this and don't have any questions - thanks for making logical commits and useful commit messages. I assume since I'm on assignee and not reviewer, you're adding another person, but it all makes sense to me. |
|
I'm not really sure of the difference between assignee/reviewer and don't really differentiate personally. Was not planning to assign anyone else :) |
|
cool, well LGTM then 👍 |
Bjvanminnen
force-pushed
the
progressFixes
branch
from
9e85db7
to
10dcaa4
Compare
Poorva did a pass of the progress redesign stuff, and came up with a big list of small things. This addresses some of them.
3. In both summary and detail view, when viewing as teacher don't make hidden/lockable stages opaque.
4. In both summary and detail view, don't disable bubbles when viewing as teacher. This way we still get the hover behavior of the bubble turning orange.