Discussion of Global State and Data Flow

[jarmoza]

This issue will serve as a central place for the planning and discussion about how data moves through the tool and where/how it is stored.

We began this discussion today moving through the current state of the landing and categorization pages and made a new sketch regarding the data flow and tool interactions. It can be found here: https://miro.com/app/board/uXjVOJWayKY=/

There are several changes to the current implementation, most involve moving around where code gets executed and rearranging the global data store into a more flat object. (No more page specific data structures, unless absolutely necessary.)

One of the bigger changes is moving away from keeping track of UI element color to signify a link between data column and category and towards just using category with a separate mapping between category and a CSS class (containing styling like color). We agreed generally on the solution outlined in the suggested changes for the recent PR that amended how colors were being selected and preserved page state: https://github.com/metaneuro/annotation_tool/pull/61.

Implementing the new global store as well as that latter implementation change will be spun out in several issues from the discussion here.

[jarmoza]

What's next: (1) A code block proposal for the store's state object (2) Assigning the new tasks for the changes on the landing and categorization pages and for the annotations.

@surchs I'll work up that code block this morning and post it here, and I suggest that I take on the work of converting the landing/categorization pages and that you begin work on state for the annotation page.

[surchs]

@jarmoza: thanks, that sounds good. I'd say: let's agree on the plan together and then discuss splitting the work.

One thing I thought of yesterday as we're rethinking this: we also probably want to keep track in the global store of what pages are accessible. I think we already have an object that lists all possible pages, so this could be as easy as adding another key to each entry that reads: accessible: bool. And then pages can read and write from it.

[jarmoza]

One thing I thought of yesterday as we're rethinking this: we also probably want to keep track in the global store of what pages are accessible. I think we already have an object that lists all possible pages, so this could be as easy as adding another key to each entry that reads: accessible: bool. And then pages can read and write from it.

Sounds good. Below is my proposal for the store based on our discussion/sketch yesterday (accessibility included).

export const state = () => ({

    pageNames: {

        home: {...},
        categorization: {...},
        annotation: {...},
        download: {...}
    },

    pageAccessiblity: {

        home: true,
        categorization: false,
        annotation: false,
        download: false
    },

    // Participants.tsv file data
    tsvFile: null,

    // Original data dictionary file data
    // (formerly home.jsonFile)
    dataDictionaryOriginal: null,

    // User-amended data dictionary file data
    dataDictionaryAmended: null,

    // Hardcoded list of categories used on the categorization page
    // and possibly elsewhere in the tool
    // formerly (categorization page's recommendedCategories)
    categoryList: [],

    // Mapping showing user-applied mappings between our categories
    // and the columns in their tsv/data dictionary file(s)
    // Keyed on column
    // (formerly categorization.paintingData)
    categoryColumnMap: {
        "age": "Age",
        ...
    },

    categoryToStyleClassMap {
        "Subject ID": "cssStyleClass1",
        ...
    },

    // Stores table data in format ready for Bootstrap table
    tsvTableDataFormatted: {}

    },

    toolColorPalette: {
        color1: "cssStyleClass1",
        ...
    },

    // Possibly more global store data for subsequent pages that needs
    // to be shared
    ...
})

[jarmoza]

One idea I had on the problem of the data dictionary data being displayed on the categorization as having two potential data sources is to give the component the ability to listen to both (dataDictionaryOriginal and dataDictionaryAmended above) and display both. We could add an extra column to indicate the new/amended data.

[jarmoza]

Some minutes from this morning's call:

1) I'll be doing the conversion of the landing and categorization page (iterative edits instead of a rewrite), and Seb will contribute discussion on the PR/continue work on the annotation page and its components. The new store from my conversion will be adjusted as needed for the annotation page. 2) Vue3/Nuxt3 upgrade will be done at a later date in an experimental branch as time allows - An issue should be filed for this task 3) Component communication with the store will stick with the current model of dispatching events to their parent containers who will then commit new data from their component to the global store (via a dispatch call to the appropriate store action method).

[surchs]

This all sounds very good and exciting! One exception to the principle of parent-talks-to-store might be the navigation component. I'm wondering how we set that up. One idea could be that it listens to the store.pageAccessibility variable to stay in sync with what links are active/inactive and the store.pageNames variable to populate these links in the first place. Then the current page name could be supplied from the parent. What do you think @jarmoza ?

[jarmoza]

The current props for the navbar are props: ["navItemsState", "pageName"] so not much will change. In this case, navitemsState will not need to be passed in and store.pageAccessibility will be referenced by the navbar component instead. When some user interactions change the state of the current page as to open access to the next page store.pageAccessibility will be modified by the page code (via store action method). Probably the navbar's navitems' disabled attribute will be tied to a computed method referencing store.pageAccessibility

[surchs]

One thing I am noticing while working on the annotation page components: it would be really helpful to have some basic documentation on the expected structure of the global store variables. Can you add one example of an actual entry for all of these variables you are proposing? We could then take the proposal and for now put it inside the wiki (and then keep it up to date there as well).

This will help a lot with making it easier to later hook up the components to the actual global store.

[jarmoza]

@surchs Here's a more fleshed out version of the store JS. Keep in mind that we will/should be accessing the store via getters.

export const state = () => ({

    // Page-related data

    pageData: {

        home: {

            accessibility: true,
            fullName: "Home",
            location: "/",
            pageName: "index",
        },

        categorization: {

            accessibility: false,
            fullName: "Categorization",
            location: "categorization",
            pageName: "categorization"
        },

        annotation: {

            accessibility: false,
            fullName: "Annotation",
            location: "annotation",
            pageName: "annotation"
        },

        download: {

            accessibility: false,
            fullName: "Download",
            location: "download",
            pageName: "download"
        }
    },

    // TSV file

    // Participants.tsv file data
    // For format see 'convertTsvLinesToDict' in index.js
    tsvDataOriginal: null,

    // Stores table data in format ready for Bootstrap table
    // This is an array of objects. See 'tableDataFromTsvAndOrJson' in
    // categorization.vue for exact format
    tsvDataTableFormatted: [

    ],

    // Data dictionary

    // Original data dictionary file data
    // (formerly home.jsonFile)
    dataDictionaryOriginal: null,

    // User-amended data dictionary file data
    dataDictionaryAmended: null,

    // Hardcoded list of categories used on the categorization page
    // and possibly elsewhere in the tool
    // formerly (categorization page's recommendedCategories)
    categoryList: [

        "Subject ID",
        "Age",
        "Sex",
        "Diagnosis",
        "Assessment Tool"
    ],

    // Mapping showing user-applied mappings between our categories
    // and the columns in their tsv/data dictionary file(s)
    // Keyed on column. If a column is not a key, it has not yet been linked to
    // a category by the user
    // (formerly categorization.paintingData)
    categoryColumnMap: {
        "age": "Age",
        ...
    },

    // Maps our categories in 'categoryList' to colors in 'toolColorPalette'
    // (Final class names pending). This way colors can be swappable and
    // rearrangeable for categories
    categoryToColorMap {
        "0": "color1",
        "1": "color2",
        "2": "color3",
        "3": "color4",
        "4": "color5",
        "-1": "colorDefault"
    },

    // Map of the tools colors to CSS classes containing color (and possibly
    // other style) values. More palettes could be defined here, either out of
    // user preference or if we ever decided to code a light/dark mode feature
    toolColorPalette: {
        color1: "cssStyleClass1",
        color2: "cssStyleClass2",
        color3: "cssStyleClass3",
        color4: "cssStyleClass4",
        color5: "cssStyleClass5",
        colorDefault: "cssStyleClassDefault"
    },

    // Possibly more global store data for subsequent pages that needs
    // to be shared (e.g. 'harmonizedTableData')
    ...
})

[surchs]

copying this conversation back here: Can we have computed properties or something effectively similar in the store? We need to end up with an object of the form

categoryToColorMap {
  "category": "color1"
  ...
}

and we start with two lists: categoryList = ['Age', 'Sex', ...] and colorList = ['color1', 'color2', 'color3', ...] (or something like it). Then if we don't want to repeat all that stuff for categoryToColorMap, we could use a computed property of the form:

categoryToColorMap() {
  const catColMap = {}
  for ([category, color] in zip(categoryList, colorList) {
    catColMap[category] = color
  }
  return catColMap
}

or whatever the correct JS expression for this is. Provided of course that computed properties exist in the global state. The reason I am bringing this up is because this is more legible to me than the stringified indices

[jarmoza]

Here's how we can do "computed" properties with the Vuex/Nuxt store.

Getters don't necessarily directly reference store data, though they can be a normal get function. We can make a categoryToColorMap getter like you suggest. That would eliminate the redundant store field. Though, of course, using the field version instead of the getter will be faster.

[surchs]

Here is a thought: eventually we may want to support other annotation formats (e.g. full-spec BIDS, DANDI, NIDM, ...). So there could be a page before all of our pages to select your "flavour" of annotation (ours would e.g. be "Bagel"). On this page, we could have a method that, as soon as the flavour is selected, "gets" the categoryList (that may be different for each flavour) and the colorList (that probably will be the same), and then "sets" the mapping between the two back to the global store.

Until we have this new first page, we could just have this method live on our current first page (where we upload the .tsv and .json files) and have it hook into an early lifecycle event (e.g. onMounted). That would allow us to both have this data in the global store, not repeat ourselves in the global store, and have a place where the mapping is explicitly generated (although the store and the creating of the mapping would be in different places).

[jarmoza]

Would this be on the "instructions" page we mentioned earlier?

[surchs]

Yeah, it could be there. Something like:

1. Instructions and select your flavour
2. Upload your data (this page would already depend on your flavour)
3. Categorize your tabular data columns (e.g. NIDM could stop here)
4. Annotate your values
5. Download your results

And so the mapping of categories to colors could happen via that selection of flavours on page 1. If you think this is a more involved decision, we could also discuss this in a separate issue. I think whichever way we go should be the one that clearer and easier to understand, even if that means we have to trade some lines of code or efficiency.

[jarmoza]

I think we should be wary of spending time reimplementing the same functionality for the sake implementation preference, but if it extends to further functionality like annotation flavor selection then it makes sense.

Having redundant information in the store, for instance, is not optimal in the ideal, abstract sense but if it allows pieces of code to operate without the need to translate from a different configuration of the same data then it is optimal in sense of efficiency and less complicated code.

[jarmoza]

And so the mapping of categories to colors could happen via that selection of flavours on page 1.

This makes sense. I'll keep the categoryToColorMap populated as is for now and we can dynamically populate it on the instructions page.

[surchs]

I'm thinking about the data flow that we discussed! We said that setters for the global state should be called from the pages, and components should not access the global state directly. That does mean that where we have nested components, there is a bit of a relay-race of data being passed down the chain, and then bubbling up the chain. Example:

Age annotation: Information on the columns annotated as "about" age is stored in the global state in categoryColumnMap. So the data flow here would be:

global.state.categoryColumnMap 
-> <page.Annotation>.getter()
-> <component.Age :annotatedColumns>.filterColumns() // to make sure we only pass on columns "about" age
-> <component.columnTable :annotatedColumns>

And then if I make any changes in the columnTable (i.e. "delete" a column because it isn't "about" age), then this change needs to bubble back up the chain via $emit until we reach the annotation page where we call a "setter" for the global store.

So in short, if I have a sub-sub-component that shows the columns that are about "age" on the annotation page and I remove one of these columns via the component, then in order for this component to update the list of columns it shows me, the information will first travel up the full hierarchy to the global state, and then travel back down the full hierarchy to my component before this component gets re-rendered.

Not saying there is anything wrong with this, just want to check here whether this is the way we want to implement it.

[jarmoza]

That's why I was hesitating and presenting that devil's advocate side yesterday. If that bubbling up system becomes too complex, we either might consider an exception or abandoning the rule. The idea of the pages handling all store changes is nice, but if it gets in the way of understandable code then that's not good either.

[jarmoza]

And like I mentioned, the Vuex/Nuxt store maintains a stub on the call chain when an action is called.

[jarmoza]

One thing I hadn't considered though is that it's possible that a parent's parent could just listen for the event. I have no tried that. Might be something to try. That would bypass the multistep bubble up.

[surchs]

I don't understand. What alternative are you proposing that is less complex and more legible than the components: $emit / prop and pages: getter() / setter() system?

[jarmoza]

Either try having the grandparent watch for the grandchild's event or just called the store action directly from the grandchild.

[surchs]

having the grandparent watch for the grandchild's event

do you have some example and link to docs for this?

edit: and can you also explain how you think this makes the structure clearer? If I understand this plan right, with 3 layers, the middle layer would not emit an event or listen for one, and instead would be bypassed by some "direct listening" setup. How is that clearer?

[jarmoza]

What I would do:

// sub-sub-component method
this.$emit("grandchild-event-name", eventData);

<!-- In page (grandparent) whose template holds sub-component -->
<sub-component v-on:grandchild-event-name="handleEvent($event)"></sub-component>

[jarmoza]

If I understand this plan right, with 3 layers, the middle layer would not emit an event or listen for one, and instead would be bypassed by some "direct listening" setup. How is that clearer?

Depending on how nested the component is, there's (at least) one less listener and emitter. And it follows the model we had agreed on yesterday.

Like I said, I'm not sure if it works. Just suggesting you try it before having the child call the store.

[surchs]

Oh, I see. I thought you had used this before. As far as I can tell, you cannot listen to events from grand-children. You can hack your way around it, but this is discouraged. See e.g. here: https://forum.vuejs.org/t/recommended-approach-to-emit-an-event-from-a-nested-child/10197/9

I have used a bubble-up, prop-down chain for the annotation page. I think it is reasonably explicit. For now I think we can use it like this.

[surchs]

@jarmoza: I came across this section of the docs (in Vue3, but Vue2 is similar) on prop-drilling as an anti-pattern: https://vuejs.org/guide/components/provide-inject.html#prop-drilling. The proposed alternative is the provide/inject tools. I think we should take a look at this section and see if we should adopt that in a future iteration over what we currently have.

I think this is essentially what you had suggested as "grandchild prop access".

[surchs]

@jarmoza: Let's keep track of the input / output of the store, pages, and components here: https://docs.google.com/document/d/1620gmKEtdfSKU4xZOAXnnxTMv5TguIk3FrD3VlwtOYE/edit?usp=sharing

Ideally also add a brief example with each!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.