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

Revert bubble choice progress tab updates that weren't ready to go out #35180

Merged
merged 2 commits into from Jun 5, 2020

Conversation

Erin007
Copy link
Contributor

@Erin007 Erin007 commented Jun 5, 2020

I accidentally merged #34986 via the command line and then created a big ol' knot and couldn't revert cleanly. See this discussion for details.

I used git checkout <commit> -- <files> to create this PR that reverses the changes in #34986

@Erin007 Erin007 requested a review from a team as a code owner June 5, 2020 21:13
@Erin007 Erin007 changed the title Revert bubble choice Revert bubble choice progress tab updates that weren't ready to go out Jun 5, 2020
@Erin007 Erin007 merged commit a016eeb into staging Jun 5, 2020
@Erin007 Erin007 deleted the revert-bubble-choice branch June 5, 2020 21:23
Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

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

🎉 👍

Copy link

@clareconstantine clareconstantine left a comment

Choose a reason for hiding this comment

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

Nice job getting this over the finish line! The screenshots look great!

if (levels[levelIndex].sublevels) {
// TODO: import SMALL_DOT_SIZE from progressStyles
// TODO: make consistent with multiGridConstants
width = width + levels[levelIndex].sublevels.length * 18;

Choose a reason for hiding this comment

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

If these items are yet to be done, is this work captured anywhere else? If it's done, we probably don't need the comments :)

@@ -168,39 +168,6 @@ def user_summary(user)
# if we have a contained level or BubbleChoice level, use that to represent progress
level = Level.cache_find(level_id)
sublevel_id = level.is_a?(BubbleChoice) ? level.best_result_sublevel(user)&.id : nil
if level.is_a?(BubbleChoice) # we have a parent level

Choose a reason for hiding this comment

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

it would be awesome to DRY this up by extracting lines 174-200 and 206-232, which are very similar, into a helper function.

if completion_status == LEVEL_STATUS.not_tried
# for now, we don't allow authorized teachers to be "locked"
if locked && !user.authorized_teacher?
levels[level_id] = {

Choose a reason for hiding this comment

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

should this be sublevel_id?

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