Information Architecture Refactor: Update the Plan > Lesson Summary #12730

AlexVelezLl · 2024-10-22T13:21:42Z

Summary

Update the Plan > Lesson Summary page to reflect both planning and report elements.

References

Closes #12707

Reviewer guidance

Does the coach plan lessons follows the behaviour described in the specs?

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

github-actions · 2024-10-22T13:54:06Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.18.0.dev0_git.20241025040829.pex
Windows Installer (EXE)	kolibri-0.18.0.dev0+git.20241025040829-windows-setup-unsigned.exe
Debian Package	kolibri_0.18.0.dev0+git.20241025040829-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.18.0.dev0+git.20241025040829.dmg
Android Package (APK)	kolibri-0.18.0.dev0+git.20241025040829-0.1.4-debug.apk
TAR file	kolibri-0.18.0.dev0+git.20241025040829.tar.gz
WHL file	kolibri-0.18.0.dev0+git.20241025040829-py2.py3-none-any.whl

marcellamaki · 2024-10-22T17:37:40Z

@radinamatic @pcenov - if we can get QA review here, that would be great. Peter, the focus can be on overall alignment to the spec. Radina, a focus on any accessibility regressions in particular would be helpful! Thank you :)

pcenov · 2024-10-23T13:34:32Z

Hi @AlexVelezLl, I was able to identify the following minor differences with the spec:

The title of the lesson is not centered and it should be checked whether the icon and text size are ok.
I don't see the 'Made visible {date} ago' text - is this part of a separate PR?
The position of the header row of the table and also the missing underline.
The spacing between the labels and the text in the sidebar is not the same as in the Figma frame.

AlexVelezLl · 2024-10-23T18:41:12Z

Thank you @pcenov!

Let me check this.
Let me check this with the team.
I think this line can be ignored for now, as this is part of the current table and I think updating this style is out of scope for now, right @marcellamaki?
Will fix this!

AlexVelezLl · 2024-10-23T19:34:23Z

Hi @pcenov I have fixed the point 1, and point 4. I have fixed the alignment between the title and the actions, and fixed the font size.

I discussed with the team and they said that point 2 can be managed in a follow up issue to further discuss the implications of introducing it.

marcellamaki · 2024-10-23T20:26:39Z

Yes - thank you @AlexVelezLl! confirming points 2 & 3 will be managed separately. I'll prioritize code review on this tomorrow, and Peter if you are able to re-review Alex's changes for items 1 and 4, that would be helpful. Thank you both

radinamatic · 2024-10-24T12:15:04Z

No regressions or errors observed while navigating by keyboard through this new Plan >Lesson summary page. All interactive elements are focusable in a meaningful order, and actionable as expected.

However, 3 elements are missing the label (accessible text):

Options button	Remove button	Visibility toggle

radinamatic · 2024-10-24T12:16:43Z

One thing is pre-existing, and we will report separately: the issue of the UNDO button inside the snackbar that appears when a resource is removed, and it needs to be put in focus immediately after the deletion.

pcenov · 2024-10-24T12:46:59Z

Hi @AlexVelezLl - I confirm that point 1 is fixed but for point 4 what I meant is that there should be more space between the label and the text below it in the side panel. Here's a screenshot:

AlexVelezLl · 2024-10-24T15:08:15Z

Thank you @radinamatic! I just added the aria labels for the options and remove buttons. For the switch one I will create a follow up in KDS as we wll need to fix this there (learningequality/kolibri-design-system#806, #12743).

And @pcenov, I have updated the margins according to the specs:

radinamatic

Missing button labels added, margins according to specs, all issues addressed, and follow-up issue filed! 👏🏽

Excellent job, @AlexVelezLl! 💯

rtibbles

I did a quick skim, and nothing jumped out at me, but it was by no means a comprehensive review.

rtibbles · 2024-10-24T17:49:58Z

yarn.lock

@@ -2963,15 +2963,6 @@ anymatch@^3.0.3, anymatch@~3.1.2:
    normalize-path "^3.0.0"
    picomatch "^2.0.4"

-"aphrodite@git+https://github.com/learningequality/aphrodite.git":


It seems like dependabot was responsible for adding this - I am not sure why it does!

Yes, I have the sense that we have had removed this a couple of times already 😅/

marcellamaki · 2024-10-25T15:40:08Z

kolibri/plugins/coach/assets/src/views/plan/LessonSummaryPage/index.vue

+          },
+        );
+      },
+      async handleResourcesChange({ newArray }) {


All of the work here to handle the resource management within the table, while certainly implied by/part of the spec, is tough, and you did a great job with it. It's a lot more complicated than it would seem just by looking at the UI. And I really like the addition of the snackbar here and the undo action. It adds complexity but a really thoughtful implementation for the user perspective.

marcellamaki · 2024-10-25T15:45:03Z

kolibri/plugins/coach/assets/src/views/reports/ReportsLessonResourcesTable.vue

    computed: {
      ...mapState('classSummary', { className: 'name' }),
    },
    methods: {
-      handleDragStart() {
-        // Used to mitigate the issue of text being selected while dragging


the upgrade to using @mousedown.prevent here is a small change but so nice

marcellamaki

Nice work @AlexVelezLl - this looks great, no concerns on my end, and some of the work here is just really thoughtful implementation and interpretation of the spec. Possibly you may end up wanting to make some further tweaks on this as the IA work wraps up, but that's fine with me, and we can continue any discussions about expected behavior as the work progresses. Nicely done!

github-actions bot added DEV: backend Python, databases, networking, filesystem... APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend labels Oct 22, 2024

marcellamaki self-requested a review October 22, 2024 13:46

marcellamaki mentioned this pull request Oct 22, 2024

Update mainline navigation and remove unused components and files after "landing pages" are merged #12729

Closed

6 tasks

marcellamaki added this to the Kolibri 0.18: Coach Resource Discovery milestone Oct 22, 2024

rtibbles assigned marcellamaki Oct 22, 2024

marcellamaki requested a review from radinamatic October 22, 2024 17:36

radinamatic approved these changes Oct 24, 2024

View reviewed changes

rtibbles reviewed Oct 24, 2024

View reviewed changes

AlexVelezLl added 10 commits October 24, 2024 23:07

Check tableHasRows for transition-groups

85eda61

Make report lessons resources table editable

6be46d7

Rename ReportsLessonResourcesTable

9971b4a

Introduce lessonResourcesTable in lessonSummaryPage

0e577b6

Add functionality to sort and delete resources

bb43096

Fix resources backup

e0ecd5e

ExportCSV for lesson resources table

385444d

Add learners table

d645af9

Modify lesson status and reorganize lesson header

5417769

Fix text selection

1bb008b

AlexVelezLl added 5 commits October 24, 2024 23:08

Avoid reload on tab change and show more options in dropdown menu

73c08fa

Fix styles

db54904

Add missing aria labels

638b7b2

Fix status style

fce7174

Fix status style

453048c

AlexVelezLl force-pushed the update-lesson-coach branch from 2be5a1a to 453048c Compare October 25, 2024 04:08

marcellamaki reviewed Oct 25, 2024

View reviewed changes

marcellamaki approved these changes Oct 25, 2024

View reviewed changes

marcellamaki merged commit 9f045f7 into learningequality:develop Oct 25, 2024
34 checks passed

AlexVelezLl deleted the update-lesson-coach branch October 25, 2024 17:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Information Architecture Refactor: Update the Plan > Lesson Summary #12730

Information Architecture Refactor: Update the Plan > Lesson Summary #12730

AlexVelezLl commented Oct 22, 2024

github-actions bot commented Oct 22, 2024 •

edited

Loading

marcellamaki commented Oct 22, 2024

pcenov commented Oct 23, 2024

AlexVelezLl commented Oct 23, 2024 •

edited

Loading

AlexVelezLl commented Oct 23, 2024

marcellamaki commented Oct 23, 2024

radinamatic commented Oct 24, 2024

radinamatic commented Oct 24, 2024

pcenov commented Oct 24, 2024

AlexVelezLl commented Oct 24, 2024 •

edited

Loading

radinamatic left a comment

rtibbles left a comment

rtibbles Oct 24, 2024

AlexVelezLl Oct 24, 2024 •

edited

Loading

marcellamaki Oct 25, 2024

marcellamaki Oct 25, 2024

marcellamaki left a comment

Information Architecture Refactor: Update the Plan > Lesson Summary #12730

Information Architecture Refactor: Update the Plan > Lesson Summary #12730

Conversation

AlexVelezLl commented Oct 22, 2024

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

github-actions bot commented Oct 22, 2024 • edited Loading

Build Artifacts

marcellamaki commented Oct 22, 2024

pcenov commented Oct 23, 2024

AlexVelezLl commented Oct 23, 2024 • edited Loading

AlexVelezLl commented Oct 23, 2024

marcellamaki commented Oct 23, 2024

radinamatic commented Oct 24, 2024

radinamatic commented Oct 24, 2024

pcenov commented Oct 24, 2024

AlexVelezLl commented Oct 24, 2024 • edited Loading

radinamatic left a comment

Choose a reason for hiding this comment

rtibbles left a comment

Choose a reason for hiding this comment

rtibbles Oct 24, 2024

Choose a reason for hiding this comment

AlexVelezLl Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

marcellamaki Oct 25, 2024

Choose a reason for hiding this comment

marcellamaki Oct 25, 2024

Choose a reason for hiding this comment

marcellamaki left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 22, 2024 •

edited

Loading

AlexVelezLl commented Oct 23, 2024 •

edited

Loading

AlexVelezLl commented Oct 24, 2024 •

edited

Loading

AlexVelezLl Oct 24, 2024 •

edited

Loading