WIP: Refactor Voter application layout

[gogarufi]

WHY:

Switching to a more conventional Svelte "layout -> page" model.

This allows us to:

• avoid re-renders of the entire layout while switching between pages
• run animations for certain transitions (progress bar, top bar)

TODO:

• generate new json settings for backend
• stop using old Page properties
• add documentation for new Svelte components

What has been changed (if possible, add screenshots, gifs, etc. )

Check off each of the following tasks as they are completed

• I have reviewed the changes myself in this PR. Please check the self-review document
• I have added or edited unit tests.
• I have run the unit tests successfully.
• I have run the e2e tests successfully.
• I have tested this change on my own device.
• I have tested this change on other devices (Using Browserstack is recommended).
• I have tested my changes using the WAVE extension
• I have added documentation where necessary.
• Is there an existing issue linked to this PR?

Clean up your git commit history before submitting the pull request!

[gogarufi]

If you remember we discussed how to change CSS of a parent from a child. There were two options, use an advanced :has CSS selector to modify drawer styles from within candidate page or setup a global context with CSS settings. The problem is that the context needs to be "reset" to default values once we left that candidate page. So this line takes care of it in a centralized way. The alternative was to call reset on the context in every page (which would be unnecessarily boilerplayty in this case) or use onDestroy on that page, but it is not guaranteed to be called always (as far as I understood) so here we go.

This whole thing is ofc necessary because we use downstream layout -> page setup instead of building pages from "lego-blocks" such as components and slots (where a new CSS value could be just passed as a property). Hopefully something in Svelte 5 can provide a better solution for us 🤞

[kaljarv]

To me it seems onDestroy is called rather consistently, but did you find some info stating otherwise? If we can leverage that, we could use it pretty easily make all page-style and header changes specific to the scope (with support for both layouts and pages).

One possibility would be even to make passing the component’s onDestroy handler a prerequisite for accessing the context (I think it'd be best to combine the various contexts into one anyway), e.g.:

// In app.context.ts
// This would be a bit more involved but it'd make life easy on the consumer side

// Push any merged styles to this stack
const styleStack: Array<PageStyles> = [ DEFAULT_SETTINGS ];

// Merge styles and add to stack
function setPageStyles(styles: DeepPartial<PageStyles> = {}): void {
  const current = styleStack[styleStack.length - 1];
  const newStyle = lodash.merge(lodash.cloneDeep(current), lodash.cloneDeep(styles));
  styleStack.push(newStyle);
  // Apply the style here
}

// Use this to move back to a previous state, unless an earlier layout was already unmounted an we already spliced the stack
function resetStack(index: number): void {
  if (styleStack.length < index + 1) return;
  styleStack.splice(index + 1);
  // Apply the style at styleStack[index]
}

// The onDestroy is used to reset the stack to where it was
export function getAppContext(onDestroy: (fn: () => unknown) => void): AppContext {
  const currentIndex = styleStack.length - 1;
  onDestroy(() => resetStack(currentIndex));
  return { setPageStyles }; // And other functions, stores etc.
}

// On a consuming +page/layout.svelte

import { onDestroy } from 'svelte';
import { getAppContext } from '$lib/contexts/app';

const { setPageStyles } = getAppContext(onDestroy);
setPageStyles({ drawer: { background: 'bg-base-300' } });
// These will reset to the previous value when unmounted

[gogarufi]

Now looking at it again, onDestroy should actually work in all cases we may need it, I initially dismissed it more for the reason of being page specific, wanted a centralized layout-based way to reset the CSS but the idea to pass it as a callback would cover that 👍 .

[gogarufi]

Ok, I am confused. I am not sure what you have in mind here (even though you brought up an example). Let's discuss and see if maybe you making the change in a separate commit is more efficient than me trying to adapt your vision :)

[kaljarv]

Hi! I know it's still WIP but already added some preliminary thoughts, but we can go through these on Mon. Looks good anyway!

[kaljarv]

We'd actually only want to show this on the /questions route. Perhaps we need to consider a different schema for defining which buttons to show. Maybe smth in the style such that if the value for an action is not defined, the default from settings will be used.

type TopBarActionsSettings = {
  [action: TopBarAction]?: 'hide' | 'show';
}

type TopBarAction = 'return' | 'help' | 'feedback'; // etc.

[gogarufi]

I am not actually sure. So we have internal app "state management", which basically makes sure that on certain pages we need to support "showing" something. If the state is set to "show" - then we need to check user settings to see if they want the element to be there or not. I think it is an "AND" relationship here.

I only added the types but they are not partial.

[kaljarv]

There might well be a better way than that I proposed, but it would be nice to be able to do these things:

1. Show feedback and help buttons on all routes if the settings say so
2. Hide either feedback or help by any layout or page
3. Show the results button on all /questions routes (i.e. defined by a layout) if the settings say so
4. Show the logout button in the Candidate App only, i.e. on all /candidate routes

Then we could do the following:

1. Banner doesn't show any actions by default
2. /+layout.svelte sets the top bar actions: { help: $settings.header.showHelp ? 'show' : undefined, feedback: $settings.header.showFeedback ? 'show' : undefined }
3. /questions/+layout.svelte sets: { results: $settings.questions.showResults ? 'show' : undefined }
4. /results/[entityType]/[entityId]/+page.svelte sets: { help: 'hide', feedback: 'hide', return: 'show' }
5. /candidate/+layout.svelte sets: { logout: 'show' }

[kaljarv]

To me it seems onDestroy is called rather consistently, but did you find some info stating otherwise? If we can leverage that, we could use it pretty easily make all page-style and header changes specific to the scope (with support for both layouts and pages).

One possibility would be even to make passing the component’s onDestroy handler a prerequisite for accessing the context (I think it'd be best to combine the various contexts into one anyway), e.g.:

// In app.context.ts
// This would be a bit more involved but it'd make life easy on the consumer side

// Push any merged styles to this stack
const styleStack: Array<PageStyles> = [ DEFAULT_SETTINGS ];

// Merge styles and add to stack
function setPageStyles(styles: DeepPartial<PageStyles> = {}): void {
  const current = styleStack[styleStack.length - 1];
  const newStyle = lodash.merge(lodash.cloneDeep(current), lodash.cloneDeep(styles));
  styleStack.push(newStyle);
  // Apply the style here
}

// Use this to move back to a previous state, unless an earlier layout was already unmounted an we already spliced the stack
function resetStack(index: number): void {
  if (styleStack.length < index + 1) return;
  styleStack.splice(index + 1);
  // Apply the style at styleStack[index]
}

// The onDestroy is used to reset the stack to where it was
export function getAppContext(onDestroy: (fn: () => unknown) => void): AppContext {
  const currentIndex = styleStack.length - 1;
  onDestroy(() => resetStack(currentIndex));
  return { setPageStyles }; // And other functions, stores etc.
}

// On a consuming +page/layout.svelte

import { onDestroy } from 'svelte';
import { getAppContext } from '$lib/contexts/app';

const { setPageStyles } = getAppContext(onDestroy);
setPageStyles({ drawer: { background: 'bg-base-300' } });
// These will reset to the previous value when unmounted

[kaljarv]

NB. We need to use the total number of questions in the selected categories for the max value.

[gogarufi]

The progress bar includes all questions from all categories as steps, it pauses on the category page and then proceeds. At least this is how it worked before the refactoring, basically this preserves the behaviour. :)

Or do you mean that we want to change that to indicate how many left in the current category and then reset to 0 and so no?

[kaljarv]

The difference is that currently we use the sum of the questions in the categories the user has selected for answering (on /questions/+page.svelte) instead of all questions. The list of selected questions is calculated on /[questionId]/+page.svelte: ~87:

  $: Promise.all([$opinionQuestions, $opinionQuestionCategories]).then(
    ([qq]) => (questions = filterAndSortQuestions(qq, $firstQuestionId, $selectedCategories))
  );

And questions.length used for the total.

[gogarufi]

Still not getting what's wrong with this logic. Sorry :D Let's discuss it on Monday.

[kaljarv]

But when we do, we just need to either use one stack with a more complex type for all changes or enforce consistent indexing between the stacks.