Updates "Section Settings" Side panel UI by AllanOXDi · Pull Request #13239 · learningequality/kolibri

AllanOXDi · 2025-03-21T13:53:35Z

Summary

This PR updates the section title in the edit section UI

Closes #13107

After

Before

References

#13107

Reviewer guidance

See https://www.figma.com/design/lVJt5ukOrxS9qay0rXy7GC/0.18-Coach-updates?node-id=107-20607&p=f&t=VBq8IMW8iRQB1p7u-0
Navigate to Create quiz
Click on the Options button
Select the edit section

github-actions · 2025-03-21T14:26:35Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.18.1.dev0_git.20250616162023.pex
Windows Installer (EXE)	kolibri-0.18.1.dev0+git.20250616162023-windows-setup-unsigned.exe
Debian Package	kolibri_0.18.1.dev0+git.20250616162023-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.18.1.dev0+git.20250616162023.dmg
Android Package (APK)	kolibri-0.18.1.dev0+git.20250616162023-0.1.4-debug.apk
Raspberry Pi Image	kolibri-pi-image-0.18.1.dev0+git.20250616162023.zip
TAR file	kolibri-0.18.1.dev0+git.20250616162023.tar.gz
WHL file	kolibri-0.18.1.dev0+git.20250616162023-py2.py3-none-any.whl

nucleogenesis

I note a few differences to the Figma but I think they're likely not binding requirements here - but noting in case @jtamiace thinks otherwise.

The Figma doesn't show a border-line beneath the side panel title
The Y-padding for the buttons footer is a bit taller than is currently implemented (FWIW I think the current implementation looks good as-is but also I played part in making it so I may be biased 😅)

nucleogenesis · 2025-03-24T15:41:50Z

        maxQuestionsLabel,
        // i18n
        displaySectionTitle,
-        sectionSettings$,


I did a quick grep - looks like this string is no longer used anywhere with it being removed from here so we could probably remove it from the quiz strings file.

jtamiace · 2025-03-25T20:57:57Z

Agreed, there shouldn't be a border beneath the title section, and there also shouldn't be a border on the bottom bar. I think the border under the title section is currently implemented in different side panels across the app and it'd be good to remove that at some point

akolson

The changes seem to make sense to me. I'll leave the final approval to @nucleogenesis @marcellamaki whenever they are ready. Thanks @AllanOXDi

AllanOXDi · 2025-04-01T19:08:22Z

HI @jtamiace does this change looks good to you?

marcellamaki

Hi @AllanOXDi - this is definitely a good start, but there are a few other changes in terms of spacing in the Figma that I think would be good to include while we're here. At the minimum, the title should be aligned with the content to the left, as seen in the figma. Currently, it's a bit off.

Also in the figma, the spacing just as a bit more "breathing room". You can see that the spacing is just a little bit larger that what we currently see in the UI. This isn't essential, but it's a perfect opportunity to practice "pixel-perfect" (or close to it) css refinement.

marcellamaki · 2025-04-10T18:05:10Z

    padding: 1em;
    margin-top: 1em;
    background-color: #ffffff;
-    border-top: 1px solid black;


While we don't want the solid black border here, the figma shows a dropshadow, so we should replace with that, rather than just remove the border

marcellamaki

HI @AllanOXDi - thanks for making the spacing updates on the side. Visually it looks better in the browser.

One blocking problem is that this change is causing a regression with the other section side panel which edits the section order.

What we need to do is:

keep a consistent design pattern for the title
not have multiple headers
make sure that in the other side panel, the user knows they are editing the order

I think there are several ways to accomplish this - maybe you can suggest an option that seems technically feasible and show Jessica what it looks like for her 👍

One non-blocking request is that you revisit some of the other spacing/layout updates in the figma, such as the space between elements, etc. that I mentioned. If you're not sure exactly what's in scope here after reviewing the Figma, please ask and we can look at it together.

marcellamaki · 2025-05-07T13:43:01Z

As a secondary thought - if you run into challenges with this (updating the CSS styles, the title/header patterns, etc.), this is really useful information for our side panel refactoring work! You can share that as part of our conversations

AlexVelezLl

Thanks @AllanOXDi! I just left some minor observations, mostly just nitpicks, but the i18n one I think its something that we must solve before merging this pr, please feel free to ask any question if needed :).

AlexVelezLl · 2025-06-12T00:53:50Z

+        v-if="route.name === PageNames.QUIZ_SECTION_ORDER"
+        class="sidepanel-title"
+      >
+        {{ coreString('editAction') }}


Just noting that we are changing from "Edit - Section order" to "Edit section order". Is this an intentional change? If we want the "Edit section order" string, I dont think that doing coreString('editAction') + sectionOrderLabel$().toLowerCase() would be the best way to achieve it, since many languages can have different grammar and have, lets say, the verb at the end of the sentence. So my best guess would be to return to the "Edit - Section order" if we dont want to introduce any new strings for this PR (since its targeted for the planned patch 2).

AlexVelezLl · 2025-06-12T01:09:10Z

        required: false,
        default: false,
      },
+      addBottomBorder: {


@marcellamaki another discrepancy in the side panels 😆. In the 0.18 designs we didn't have a separator line below the header in any of the frames in figma. But in the implementation of this component, we do have this line. I think for consistency we should have or should not have this line for all the side panels of at least the same section. So either option, or we show this line for the SectionSidePanel component, or we also remove this line from the resource selection side panel (or we remove this line within this component for all side panels), what do you think?

AlexVelezLl

Just a final thought 😄, after this, this looks good to me! Will defer the final approve and merge to @nucleogenesis or @marcellamaki.

AlexVelezLl · 2025-06-13T17:04:50Z

+        class="sidepanel-title"
+      >
+        {{ editAction$() }} -
+        {{ sectionOrderLabel$().toLowerCase() }}


I think we can to sectionOrderLabel$() instead of sectionOrderLabel$().toLowerCase() now that we have that dash again.

…l title

nucleogenesis

The underline beneath the side panel title is missing on my end but is present in the other side panels.

Otherwise code & everything else LGTM

marcellamaki · 2025-07-24T15:07:53Z

The underline should be removed, it's part of the spec (see above). I think we have some further side panel cleanup here to do for consistency, but I don't think that's in scope for @AllanOXDi 's PR here. I will need to think then if we should merge this now for 0.18 patch 2 and have it not match, or if we should merge this into develop and do all of the cleanup in 0.19

Alex's feedback addressed

AllanOXDi requested a review from nucleogenesis March 21, 2025 13:53

github-actions Bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend labels Mar 21, 2025

AllanOXDi requested review from akolson and marcellamaki March 21, 2025 13:53

rtibbles added the P1 - important Priority: High impact on UX label Mar 24, 2025

nucleogenesis reviewed Mar 24, 2025

View reviewed changes

rtibbles assigned nucleogenesis Mar 25, 2025

akolson reviewed Apr 1, 2025

View reviewed changes

marcellamaki requested changes Apr 10, 2025

View reviewed changes

marcellamaki reviewed Apr 10, 2025

View reviewed changes

marcellamaki previously requested changes May 7, 2025

View reviewed changes

AlexVelezLl reviewed May 30, 2025

View reviewed changes

Comment thread .../coach/assets/src/views/quizzes/CreateExamPage/sidePanels/SectionSidePanel/SectionEditor.vue Outdated

AllanOXDi force-pushed the update-section-settings branch from 9c2e7b8 to 5877ef6 Compare May 30, 2025 16:10

github-actions Bot added the SIZE: small label May 30, 2025

AllanOXDi requested review from AlexVelezLl and marcellamaki June 4, 2025 15:09

AlexVelezLl previously requested changes Jun 12, 2025

View reviewed changes

AlexVelezLl reviewed Jun 13, 2025

View reviewed changes

AlexVelezLl self-requested a review June 13, 2025 17:08

AllanOXDi added 6 commits June 16, 2025 18:11

adds a more distinct title for the edit section sidepanel

0ad9973

removes the sections settings from the enchancedquiz strings

999ee1e

adds a prop to enable toggling the bottom border beneath the sidepane…

8e2087e

…l title

aligns correctly the sidepanel elements

bf9dc82

conditionalizes display of sidepanel heading

033916e

use KDS dropdshadows

3d43fc9

AllanOXDi added 5 commits June 16, 2025 18:11

capitalize only the first word in the edit section section title display

0796a98

uses in template section

3ae9053

increases shadow to 2dp

934384b

adds - to separate the strings in the header

92d0000

removes conversion to lowercase

d71c39e

AllanOXDi force-pushed the update-section-settings branch from 8f68972 to d71c39e Compare June 16, 2025 16:13

rebase

c6b7a6b

marcellamaki added this to the Kolibri 0.18: Planned Patch 2 milestone Jul 24, 2025

nucleogenesis reviewed Jul 24, 2025

View reviewed changes

marcellamaki approved these changes Jul 25, 2025

View reviewed changes

marcellamaki merged commit cbc5691 into learningequality:develop Jul 25, 2025
51 checks passed

Uh oh!

Conversation

AllanOXDi commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

After

Before

References

Reviewer guidance

Uh oh!

github-actions Bot commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Artifacts

Uh oh!

nucleogenesis left a comment

Choose a reason for hiding this comment

Uh oh!

nucleogenesis Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

jtamiace commented Mar 25, 2025

Uh oh!

akolson left a comment

Choose a reason for hiding this comment

Uh oh!

AllanOXDi commented Apr 1, 2025

Uh oh!

marcellamaki left a comment

Choose a reason for hiding this comment

Uh oh!

marcellamaki Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

marcellamaki left a comment

Choose a reason for hiding this comment

Uh oh!

marcellamaki commented May 7, 2025

Uh oh!

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexVelezLl Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

nucleogenesis left a comment

Choose a reason for hiding this comment

Uh oh!

marcellamaki commented Jul 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

AllanOXDi commented Mar 21, 2025 •

edited

Loading

github-actions Bot commented Mar 21, 2025 •

edited

Loading