Make course module folding optional and implement automatic scrolling #1189

ihalaij1 · 2023-06-05T07:23:15Z

Description

What?

The automatic folding and hiding of expired course modules on the user results page was suboptimal and annoying for some students/teachers.

Old course modules are no longer hidden behind a button and folding is made optional with the use of a new "Expand all modules" button. Automatic scrolling takes the user to the first open module, unless the page contains previously unseen course news.

Why?

Improvements to course module folding were requested by a teacher.

How?

Local storage is used to save the state of the "Expand all modules" button and the course news strings for each course.

If the "Expand all modules" button is checked, all course modules will be expanded upon page load, even if they are expired or inaccessible.

If the page contains course news that aren't found in local storage, the page will not be automatically scrolled to the first open module, so that the student has a chance of noticing the news.

Fixes #1125

Testing

What type of test did you run?

Tested that the folding and scrolling work as expected.

Did you test the changes in

Chrome
Safari
This pull request cannot be tested in the browser.

Translation

Did you modify or add new strings in the user interface?

Programming style

Did you follow our style guides?
Did you use Python type hinting in all functions that you added or edited? (type hints for function parameters and return values)

Have you updated the README or other relevant documentation?

documents inside the doc directory.
README.md.
Aplus Manual.
Other documentation (mention below which documentation).

Is it Done?

Reviewer has finished the code review
After the review, the developer has made changes accordingly
Customer/Teacher has accepted the implementation of the feature

ihalaij1 · 2023-06-28T07:07:08Z

We agreed to add another button to select the scrolling behavior, because some people might want to disable it or want it to scroll immediately without animation.

ihalaij1 · 2023-06-30T11:28:42Z

I pushed some updates, ready for a review.

lainets

Great work! One proposed behavioural change: I don't think the page should scroll automatically for people new to the course. We probably want any introductory text at the top of the course be visible in that case. The simple fix is to not scroll if the first module (or, equivalently, all modules) is visible. Scrolling in that case doesn't seem necessary as the module would be visible anyway (unless the intro text is really long).

exercise/static/exercise/user_results.js

exercise/templates/exercise/_user_results.html

exercise/static/exercise/user_results.js

lainets

Nice work! In the future, it'd be preferable if any new changes came in a separate commit to make it easier to see what has changed.

exercise/templates/exercise/_user_results.html

exercise/static/exercise/user_results.js

ihalaij1 · 2023-07-19T10:16:42Z

Nice work! In the future, it'd be preferable if any new changes came in a separate commit to make it easier to see what has changed.

You can still see what changed since the last force-push if you click the Compare button next to the force-push activity.
It's easier not having to squash the commits later if everything is ready for merging.

lainets

I otherwise approve this but I'm wondering whether the previous module should be uncollapsed as stated in the issue #1125 or wheter it becomes a non-issue after these changes. I also don't see any hurry in merging this, so we could wait in case Markku or Pasi has some comments about the exact behaviour.

You can still see the what changed since the last force-push if you click the Compare button next to the force-push activity. It's easier not having to squash the commits later if everything is ready for merging.

Oh! I didn't realize. However, there is still the issue that review comments cannot be added through that compare page. In this case it wouldn't have mattered as I had no new comments to make but in general at least I would prefer having the corrections in separate commits.

ihalaij1 · 2023-07-19T11:28:48Z

I otherwise approve this but I'm wondering whether the previous module should be uncollapsed as stated in the issue #1125 or wheter it becomes a non-issue after these changes. I also don't see any hurry in merging this, so we could wait in case Markku or Pasi has some comments about the exact behaviour.

I previously talked about expanding the most recently closed module with Markku, and I think we came to the conclusion it would be a bit confusing. For example, it might look like to the student that the module is still open, since the older closed modules are collapsed.

markkuriekkinen · 2023-08-14T18:41:27Z

exercise/static/exercise/user_results.js

@@ -0,0 +1,146 @@
+function checkButton(button) {
+	button
+	  .attr("aria-pressed", "true")


This line contains mixed tabs and spaces for indentation. The file uses tabs, but our JS styleguide requires 2 spaces.

https://apluslms.github.io/contribute/styleguides/

I changed it to spaces myself.

The automatic folding and hiding of expired course modules on the user results page was suboptimal and annoying for some students/teachers. Old course modules are no longer hidden behind a button and folding is made optional with the use of a new "Expand all modules" button. Automatic scrolling takes the user to the first open module, unless the page contains previously unseen course news. Automatic scrolling can be enabled/disabled and the behavior can be selected using two new buttons. Fixes apluslms#1125

ihalaij1 requested a review from markkuriekkinen June 5, 2023 07:23

ihalaij1 modified the milestone: Summer 2023 Jun 5, 2023

ihalaij1 assigned markkuriekkinen Jun 5, 2023

ihalaij1 added area: UX teacher User experience and usability for teachers area: UX student User experience and usability for students area: user interface User interface issues that are not specifically about navigation or user experience (UX) labels Jun 5, 2023

ihalaij1 force-pushed the fix-folding branch 2 times, most recently from 5ba2eec to 18dccd7 Compare June 5, 2023 07:36

ihalaij1 force-pushed the fix-folding branch from 18dccd7 to 0201d44 Compare June 27, 2023 13:17

ihalaij1 removed the request for review from markkuriekkinen June 28, 2023 09:03

ihalaij1 assigned ihalaij1 and unassigned markkuriekkinen Jun 28, 2023

ihalaij1 force-pushed the fix-folding branch from 0201d44 to 48a2fdf Compare June 30, 2023 11:26

ihalaij1 requested a review from markkuriekkinen June 30, 2023 11:28

ihalaij1 assigned markkuriekkinen and unassigned ihalaij1 Jun 30, 2023

ihalaij1 force-pushed the fix-folding branch from 48a2fdf to bceab6e Compare June 30, 2023 12:00

lainets suggested changes Jul 17, 2023

View reviewed changes

ihalaij1 force-pushed the fix-folding branch from bceab6e to fac46d4 Compare July 19, 2023 06:07

ihalaij1 requested a review from lainets July 19, 2023 06:13

lainets suggested changes Jul 19, 2023

View reviewed changes

exercise/templates/exercise/_user_results.html Outdated Show resolved Hide resolved

exercise/static/exercise/user_results.js Outdated Show resolved Hide resolved

ihalaij1 force-pushed the fix-folding branch from fac46d4 to dbb5f1c Compare July 19, 2023 10:13

ihalaij1 requested a review from lainets July 19, 2023 10:16

lainets reviewed Jul 19, 2023

View reviewed changes

lainets approved these changes Jul 19, 2023

View reviewed changes

markkuriekkinen approved these changes Aug 14, 2023

View reviewed changes

markkuriekkinen force-pushed the fix-folding branch from dbb5f1c to 17aa6eb Compare August 14, 2023 20:23

markkuriekkinen merged commit 969800d into apluslms:master Aug 14, 2023
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make course module folding optional and implement automatic scrolling #1189

Make course module folding optional and implement automatic scrolling #1189

ihalaij1 commented Jun 5, 2023 •

edited

Loading

ihalaij1 commented Jun 28, 2023

ihalaij1 commented Jun 30, 2023

lainets left a comment

lainets left a comment

ihalaij1 commented Jul 19, 2023 •

edited

Loading

lainets left a comment

ihalaij1 commented Jul 19, 2023

markkuriekkinen Aug 14, 2023

markkuriekkinen Aug 14, 2023

Make course module folding optional and implement automatic scrolling #1189

Make course module folding optional and implement automatic scrolling #1189

Conversation

ihalaij1 commented Jun 5, 2023 • edited Loading

Description

Testing

Translation

Programming style

Have you updated the README or other relevant documentation?

Is it Done?

ihalaij1 commented Jun 28, 2023

ihalaij1 commented Jun 30, 2023

lainets left a comment

Choose a reason for hiding this comment

lainets left a comment

Choose a reason for hiding this comment

ihalaij1 commented Jul 19, 2023 • edited Loading

lainets left a comment

Choose a reason for hiding this comment

ihalaij1 commented Jul 19, 2023

markkuriekkinen Aug 14, 2023

Choose a reason for hiding this comment

markkuriekkinen Aug 14, 2023

Choose a reason for hiding this comment

ihalaij1 commented Jun 5, 2023 •

edited

Loading

ihalaij1 commented Jul 19, 2023 •

edited

Loading