CB to LB: Add UI display of Lesson Group Description and Big Questions #36298
Conversation
dmcavoy
changed the title
| @@ -660,6 +664,19 @@ export const groupedLessons = (state, includeBonusLevels = false) => { | |||
|
|
|||
| const allLevels = levelsByLesson(state); | |||
|
|
|||
| state.lessonGroups.forEach(lessonGroup => { | |||
| byGroup[lessonGroup.display_name] = { | |||
There was a problem hiding this comment.
in case two lesson groups had the same display name, I think it would make more sense to group by key rather than display name, assuming that information is available here.
| import DetailProgressTable from '@cdo/apps/templates/progress/DetailProgressTable'; | ||
| import SummaryProgressTable from '@cdo/apps/templates/progress/SummaryProgressTable'; | ||
| import FontAwesome from '@cdo/apps/templates/FontAwesome'; |
There was a problem hiding this comment.
any particular reason these have changed? Just checking if I'm missing a new best practice since I don't remember any conclusion to the debate of which method of importing things is preferred.
There was a problem hiding this comment.
Ummm I don't know if anyone else has a rule. I like it better when they are written this way because then if you want to use them somewhere else or move the file it doesn't cause problems.
| }; | ||
|
|
||
| describe('LessonGroup', () => { | ||
| it('renders clickable lesson group info button when there is a description or big questions', () => { |
|
@davidsbailey any idea why I needed to do the work in d67fbc2 to get db_query_test.rb 'test_script_level_show' to pass but it was not an issue before. I can't see where my changes would impact this. |
hard to say without more context. what is the name of the level that this was failing for without d67fbc2, and what operation was it failing during? |
If this seems odd, it's probably worth digging into to figure out what's going on, since there's some chance some bug was introduced. |
|
@uponthesun @davidsbailey Tracked it down. This change is because of my summarize lesson groups which also summarizes lessons. |
| levels_and_texts&.reject {|l| l.is_a?(External)} | ||
| end | ||
|
|
||
| def texts | ||
| levels_and_texts.select {|l| l.is_a?(External)} | ||
| levels_and_texts&.select {|l| l.is_a?(External)} |
There was a problem hiding this comment.
merging this makes me a little uncomfortable -- we have caught a number of issues where levels_and_texts is nil because it exposes that there is someplace we haven't updated our system to use the new level group format. @dmcavoy can you describe in more detail what was failing, maybe by pointing to a stack trace?
There was a problem hiding this comment.
Created PR to fix: #36559
This is part of the work to move Chapter over from CB to LB. This is a follow up to #36299 which set up the new dsl syntax for adding big questions and descriptions to lesson groups.
This adds an info icon next to the lesson group name on the script overview page if a Lesson Group has a description or big questions. This icon also shows up in the MiniView.
Clicking on the info icon pops open a dialog with the description and/or big questions.
Links
Testing story
Reviewer Checklist: