Allow teachers to hide all lessons in a unit with one click #48863

rshipp · 2022-10-26T18:45:50Z

Add two buttons to the top of the unit overview page (right above the first lesson tile) that have the option to "show all" and "hide all" with a tooltip to the right of it that says "Make all lessons in this Unit visible or hidden for your students".

The new buttons tie in to the existing logic for the toggle buttons down the rest of the page, and flip them dynamically.

Questions:

Should I add a storybook file? I looked at it briefly and it seemed like it would take some work to figure out how to hook up all the redux stuff. Or should there be any other types of tests?
Do I want the new buttons to show disabled when all the lessons are either hidden or visible? right now they feel a little low feedback when you click them
I'm making one network call per lesson in the unit, which is probably not too many, but still seems less than ideal. Is it worth adding a bulk endpoint or something to the controller so I can do one call, and adding all the JS to handle that appropriately? It seems like that might increase the scope of this task by quite a bit.
There's a warning in the console about "componentWillUpdate has been renamed, and is not recommended for use... Please update the following components: Connect(BulkLessonVisibilityToggle)" - is that something I should be concerned about here?

Links

jira ticket: TEACH-16

Testing story

Manual testing, and one new basic unit test case.

rshipp · 2022-10-26T20:32:18Z

apps/src/code-studio/components/progress/BulkLessonVisibilityToggle.jsx

+import React from 'react';
+import {connect} from 'react-redux';
+import {getStore} from '../../../redux';
+import PropTypes from 'prop-types';
+import Button from '@cdo/apps/templates/Button';
+import FontAwesome from '../../../templates/FontAwesome';
+import ReactTooltip from 'react-tooltip';
+import {toggleHiddenLesson} from '@cdo/apps/code-studio/hiddenLessonRedux';
+import {unitCalendarLesson} from '../../../templates/progress/unitCalendarLessonShapes';
+import _ from 'lodash';
+import style from './bulk-lesson-visibility-toggle.module.scss';


these are entirely unordered, and some use an absolute syntax while others use a relative syntax. are there any standards for this?

We don't have any standards for order unfortunately :(

For absolute vs relative syntax: I'll use relative if it's in the the files is in the same directory and otherwise use @cdo/apps. I'm not sure if that's just a personal preference though

rshipp · 2022-10-26T20:33:11Z

apps/src/code-studio/components/progress/BulkLessonVisibilityToggle.jsx

+  scriptName: PropTypes.string.isRequired
+};
+
+function toggleHiddenLessons(scriptName, sectionId, lessons, hidden) {


is it ok that this function is just floating around down here, or should it be somewhere else?

I normally put these at the top, but, again, not sure if that's just personal preference

rshipp · 2022-10-26T20:33:50Z

apps/src/code-studio/components/progress/BulkLessonVisibilityToggle.jsx

+function toggleHiddenLessons(scriptName, sectionId, lessons, hidden) {
+  lessons.forEach(lesson => {
+    // For some reason, sectionId is a number here, and needs to be a string
+    // for the redux toggle stuff to work.


is this related to the propTypes above? https://github.com/code-dot-org/code-dot-org/pull/48863/files#diff-df37c341b4e24afa4446ab42d92a84705737c3bc034cf85dfdcd51183bedc078R50. or is there something else I'm missing?

rshipp · 2022-10-26T20:35:05Z

apps/src/code-studio/components/progress/BulkLessonVisibilityToggle.jsx

+  }) /*,
+  dispatch => ({
+    toggleHiddenLesson(unitName, sectionId, lessonId, hidden) {
+      dispatch(toggleHiddenLesson(unitName, sectionId, lessonId, hidden));
+    }
+  }) */


i copied this from ProgressLessonTeacherInfo.jsx, but it seems to work fine without it? do I need this?

I would remove the commented out code here. Not sure why it's in ProgressLessonTeacherInfo

rshipp · 2022-10-26T20:46:37Z

apps/src/code-studio/components/progress/BulkLessonVisibilityToggle.jsx

+        <p>
+          Make all lessons in this Unit visible or hidden for your students.
+        </p>
+      </ReactTooltip>


HelpTip has a Portal here, should I use one too? i'm not sure what the benefit is

Out of curiosity, why didn't you use HelpTip here? (I'm not sure what benefit a Portal gets you either though)

It just doesn't look very good, and doesn't have a way to pass in options to override the default look. I suppose I also could have modified it to let me pass in classNames/icons stuff, would that make more sense?

I think this is fine in that case! Was just curious

bethanyaconnor · 2022-10-27T16:09:21Z

apps/src/code-studio/components/progress/BulkLessonVisibilityToggle.jsx

+import style from './bulk-lesson-visibility-toggle.module.scss';
+import i18n from '@cdo/locale';
+
+function BulkLessonVisibilityToggle({lessons, sectionId, scriptName}) {


if it's not too much trouble, it would be great to use unitName instead of scriptName. We're in the process of change the term "script" to "unit"

bethanyaconnor

A couple of questions but looks good!

hannahbergam

The question you ask about how it looks a little low feedback without disabling whichever button was just pressed- do the 'visible/hidden' fields in each lesson update automatically, or does it require a page load?

rshipp · 2022-11-01T17:14:40Z

The visible/hidden toggle for each lesson updates immediately, without a page load. They're just lower on the screen, so depending on the screen size/etc it might not be visible, or immediately clear that something happened.

rshipp added 3 commits October 26, 2022 13:40

Add hide/show all lessons buttons

a488188

Fix existing test

2aff7d0

Add new test

5216281

rshipp commented Oct 26, 2022

View reviewed changes

rshipp marked this pull request as ready for review October 26, 2022 20:50

rshipp marked this pull request as draft October 26, 2022 20:50

Extract strings for i18n

4bdb37f

rshipp force-pushed the hide-all-lessons branch from e3f419a to 4bdb37f Compare October 26, 2022 20:57

rshipp marked this pull request as ready for review October 26, 2022 20:58

rshipp requested a review from a team as a code owner October 26, 2022 20:58

rshipp requested a review from a team October 26, 2022 20:58

Hide the new buttons if unit can't hide lessons

4ddea11

bethanyaconnor reviewed Oct 27, 2022

View reviewed changes

rshipp added 3 commits October 27, 2022 11:41

Rename scriptName -> unitName

4ad8edd

Reorder functions

c8e371a

Change import syntax/order

bc291c9

bethanyaconnor approved these changes Oct 31, 2022

View reviewed changes

Remove commented dispatch code

59d8f1a

hannahbergam approved these changes Nov 1, 2022

View reviewed changes

rshipp merged commit 93f8853 into staging Nov 2, 2022

rshipp deleted the hide-all-lessons branch November 2, 2022 19:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow teachers to hide all lessons in a unit with one click #48863

Allow teachers to hide all lessons in a unit with one click #48863

rshipp commented Oct 26, 2022 •

edited

Loading

rshipp Oct 26, 2022

bethanyaconnor Oct 27, 2022

rshipp Oct 26, 2022

bethanyaconnor Oct 27, 2022

rshipp Oct 26, 2022

rshipp Oct 26, 2022

bethanyaconnor Oct 31, 2022

rshipp Oct 26, 2022

bethanyaconnor Oct 31, 2022

rshipp Oct 31, 2022

bethanyaconnor Nov 1, 2022

bethanyaconnor Oct 27, 2022

bethanyaconnor left a comment

hannahbergam left a comment

rshipp commented Nov 1, 2022

Allow teachers to hide all lessons in a unit with one click #48863

Allow teachers to hide all lessons in a unit with one click #48863

Conversation

rshipp commented Oct 26, 2022 • edited Loading

Links

Testing story

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bethanyaconnor left a comment

Choose a reason for hiding this comment

hannahbergam left a comment

Choose a reason for hiding this comment

rshipp commented Nov 1, 2022

rshipp commented Oct 26, 2022 •

edited

Loading