Implement metadata editor redesign

[gordlin]

Related Item(s)

Issues:
#374
#399

Changes

• Implemented redesign of both metadata editor pages, as well as all related functionality.

Notes

NOTE: The new product details form is not implemented in this PR, so the old one is still here. It has a separate issue: #400. Should only be done after this PR is merged, or should at least base its branch on this one.

Old 'Edit Existing' page:

New 'Edit Existing' page (before UID & load):

New 'Edit Existing' page (after UID & load; black box hidden on actual page):

Old 'Create New' page:

New 'Create New' page (black box hidden on actual page):

Testing

Steps:

1. Select any language, click onto 'Edit Existing'. Enter any existing product ID, click enter OR press the load button. Notice the very large spinner, and the info-text about loading times. Once loading's done, view the result.
2. If there's history, you'll see some text confirming it and a button to open up the history results table. If not, the text will indicate it, and the button will be hidden. The form should show the correct metadata information.
3. Go back to the New Storylines page. Try creating a new storyline with the form; it should still work.

---

This change is 

[github-actions]

Your demo site is ready! 🚀 Visit it here: https://ramp4-pcar4.github.io/storylines-editor/redesign-metadata-editor

[yileifeng]

Nice enhancements 👍

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @gordlin)

---

src/components/metadata-editor.vue line 13 at r1 (raw file):

                    </div>
                    <div class="flex flex-row md:flex-col gap-3 justify-between text-right">
                        <router-link

This should not be needed, the language toggle should be page specific so would only appear when we load the Canada.ca template. If we load the page with Canada.ca template now we would get two instances of the language hyperlink.

---

src/components/metadata-editor.vue line 38 at r1 (raw file):

                    <section>
                        <label class="font-bold">{{ $t('editor.uuid') }}</label>

Few points regarding UUID section:

• mark "(required)" text as red font
• reduce padding between UUID label and input box
• add a "clear button" to the input as per the mockups

---

src/components/metadata-editor.vue line 753 at r1 (raw file):

        } catch (err: any) {
            if (err.name === 'AbortError' || err instanceof PointerEvent) {
                Message.success(`Product retrieval manually aborted.`);

Need a translation here, unofficial Google translate marked with a 0 in the CSV file is fine. Noticed there are a few more instances of non-translations in this component so if you could fix those at the same time would be great. Also, I don't think we should display the green check in the popup message when cancelling a product load from a user perspective.

[yileifeng]

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @gordlin)

---

src/components/metadata-editor.vue line 13 at r1 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

This should not be needed, the language toggle should be page specific so would only appear when we load the Canada.ca template. If we load the page with Canada.ca template now we would get two instances of the language hyperlink.

Ah my bad revisiting the mockups it looks like they have separate view for non-Canada.ca template in which case this section is needed. However, we still need a way to conditionally render the element in case we are using the template to prevent duplicate instances.

[RyanCoulsonCA]

Reviewable status: 0 of 3 files reviewed, 11 unresolved discussions (waiting on @gordlin and @yileifeng)

---

src/components/metadata-editor.vue line 5 at r1 (raw file):

    <div class="editor-container">
        <template v-if="!loadEditor">
            <div class="">

You can remove the empty class attribute here

---

src/components/metadata-editor.vue line 173 at r1 (raw file):

                                    class="flex flex-row items-center text-yellow-500 rounded p-1"
                                >
                                    <!-- <div class="editor-label"></div> -->

You can remove this if it's unused

---

src/components/metadata-editor.vue line 239 at r1 (raw file):

                    <br />

                    <section v-if="!(editExisting && loadStatus !== 'loaded')">

I think the condition editExisting && loadStatus === 'loaded' would be cleaner here as we want to display this for existing products that have finished loading.

---

src/components/metadata-editor.vue line 325 at r1 (raw file):

                        <div class="mb-4">
                            <h1 class="text-2xl font-semibold">{{ $t('editor.productDetails') }}</h1>
                            <!-- <h3 class="editor-h3">{{ $t('editor.productDetails') }}</h3> -->

This can be removed as well

---

src/components/metadata-editor.vue line 422 at r1 (raw file):

                                @click="saveMetadata(false)"
                            >
                                Done

This is a pre-existing problem as well, but it should have a translation

---

src/components/metadata-editor.vue line 723 at r1 (raw file):

            if (res.status === 404) {
                // Product not found.
                Message.error(`The requested UUID '${this.uuid ?? ''}' does not exist.`);

Need a translation here too. Looks like this one was already hardcoded previously but we should fix it anyway!

---

src/components/metadata-editor.vue line 755 at r1 (raw file):

                Message.success(`Product retrieval manually aborted.`);
            } else {
                Message.error(`Failed to load product, no response from server`);

Translation needed here

---

src/components/metadata-editor.vue line 766 at r1 (raw file):

        // as the history should be much smaller and quicker to fetch than the config

        if (this.uuid === undefined) Message.error(`You must first enter a UUID`);

Translation needed here

[gordlin]

Reviewable status: 0 of 4 files reviewed, 11 unresolved discussions (waiting on @RyanCoulsonCA and @yileifeng)

---

src/components/metadata-editor.vue line 5 at r1 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

You can remove the empty class attribute here

Donethanks!

---

src/components/metadata-editor.vue line 13 at r1 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

Ah my bad revisiting the mockups it looks like they have separate view for non-Canada.ca template in which case this section is needed. However, we still need a way to conditionally render the element in case we are using the template to prevent duplicate instances.

Donethanks! The toggle will be hidden on the Canada.ca template and show everywhere else.

---

src/components/metadata-editor.vue line 38 at r1 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

Few points regarding UUID section:

• mark "(required)" text as red font
• reduce padding between UUID label and input box
• add a "clear button" to the input as per the mockups

Donethanks!

---

src/components/metadata-editor.vue line 173 at r1 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

You can remove this if it's unused

Donethanks!

---

src/components/metadata-editor.vue line 239 at r1 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

I think the condition editExisting && loadStatus === 'loaded' would be cleaner here as we want to display this for existing products that have finished loading.

The new condition wouldn't work, since the metadata tables won't display for the Create New Storylines page (as editExisting is not true there). Existing condition might have to stay.

---

src/components/metadata-editor.vue line 325 at r1 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

This can be removed as well

Donethanks!

---

src/components/metadata-editor.vue line 422 at r1 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

This is a pre-existing problem as well, but it should have a translation

Donethanks!

---

src/components/metadata-editor.vue line 723 at r1 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

Need a translation here too. Looks like this one was already hardcoded previously but we should fix it anyway!

Donethanks!

---

src/components/metadata-editor.vue line 753 at r1 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

Need a translation here, unofficial Google translate marked with a 0 in the CSV file is fine. Noticed there are a few more instances of non-translations in this component so if you could fix those at the same time would be great. Also, I don't think we should display the green check in the popup message when cancelling a product load from a user perspective.

Donethanks!

---

src/components/metadata-editor.vue line 755 at r1 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

Translation needed here

Donethanks!

---

src/components/metadata-editor.vue line 766 at r1 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

Translation needed here

Donethanks!

[yileifeng]

Reviewed 1 of 3 files at r1, 3 of 3 files at r2.
Reviewable status: 3 of 4 files reviewed, 11 unresolved discussions (waiting on @gordlin and @RyanCoulsonCA)

---

src/components/metadata-editor.vue line 24 at r2 (raw file):

                            }"
                        >
                            <a class="sub-link">

A bit nitpicky, but I prefer if these anchor elements were styled as buttons (no underline, darker font color) to look less like a hyperlink and more like app functionality. For reference, something along the lines of the "old" rename functionality:

Same applies to the other "sub-link" class elements below (swap config lang, click here to change UUID).

---

src/components/metadata-editor.vue line 45 at r2 (raw file):

                        <label class="font-bold mb-0"
                            >{{ $t('editor.uuid') }}
                            <span class="text-red-500 font-normal">{{ $t('editor.uuid.required') }}</span></label

Very low contrast WAVE error (3.76:1) on the (required) label.

---

src/components/metadata-editor.vue line 68 at r2 (raw file):

                                <input
                                    class="editor-input mt-0 flex-2"
                                    type="search"

Empty + missing form label when running WAVE on this search input

[RyanCoulsonCA]

Reviewable status: 3 of 4 files reviewed, 4 unresolved discussions (waiting on @gordlin and @yileifeng)

---

src/components/metadata-editor.vue line 239 at r1 (raw file):

Previously, gordlin (Gordon Lin) wrote…

The new condition wouldn't work, since the metadata tables won't display for the Create New Storylines page (as editExisting is not true there). Existing condition might have to stay.

Ah, I understand. I didn't catch that the rest of the metadata form was in this section as well. In that case maybe just replace the condition with !editExisting || loadStatus === 'loaded' since it's logically equivalent to the condition you have already have (but just removes the double negative).

[gordlin]

Reviewable status: 2 of 4 files reviewed, 5 unresolved discussions (waiting on @RyanCoulsonCA and @yileifeng)

a discussion (no related file):
Note: Added translations for all of the popup messages/warnings/infos. I think the metadata page is now fully "translated". Let me know if I missed any.

---

---

src/components/metadata-editor.vue line 239 at r1 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

Ah, I understand. I didn't catch that the rest of the metadata form was in this section as well. In that case maybe just replace the condition with !editExisting || loadStatus === 'loaded' since it's logically equivalent to the condition you have already have (but just removes the double negative).

Donethanks!

---

src/components/metadata-editor.vue line 24 at r2 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

A bit nitpicky, but I prefer if these anchor elements were styled as buttons (no underline, darker font color) to look less like a hyperlink and more like app functionality. For reference, something along the lines of the "old" rename functionality:

Same applies to the other "sub-link" class elements below (swap config lang, click here to change UUID).

Changing the link colour/removing the underline would make the "Change UUID" text very hard to distinguish from the heading. Personally I'm in favour of leaving the link style, it makes it very distinctive (and hopefully more accessible as a result).

@RyanCoulsonCA thoughts?

---

src/components/metadata-editor.vue line 45 at r2 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

Very low contrast WAVE error (3.76:1) on the (required) label.

Donethanks!

---

src/components/metadata-editor.vue line 68 at r2 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

Empty + missing form label when running WAVE on this search input

Donethanks!

[RyanCoulsonCA]

Reviewed 1 of 2 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @yileifeng)

---

src/components/metadata-editor.vue line 24 at r2 (raw file):

Previously, gordlin (Gordon Lin) wrote…

Changing the link colour/removing the underline would make the "Change UUID" text very hard to distinguish from the heading. Personally I'm in favour of leaving the link style, it makes it very distinctive (and hopefully more accessible as a result).

@RyanCoulsonCA thoughts?

I don't have much of a preference on this. I'd say just leave it as is with the blue link for now, and maybe we can throw it in a future UI/UX review.

[yileifeng]

Reviewed 1 of 2 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @gordlin and @RyanCoulsonCA)

---

src/components/metadata-editor.vue line 24 at r2 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

I don't have much of a preference on this. I'd say just leave it as is with the blue link for now, and maybe we can throw it in a future UI/UX review.

Fine with merging it as is, but will leave PR open for a bit longer in case anyone else have opinions in the next meeting.

[gordlin]

Reviewable status: 2 of 5 files reviewed, 3 unresolved discussions (waiting on @RyanCoulsonCA and @yileifeng)

a discussion (no related file):
Latest push fixes a merge conflict. There should be no differences.

---

[gordlin]

Reviewable status: 2 of 5 files reviewed, 3 unresolved discussions (waiting on @RyanCoulsonCA and @yileifeng)

a discussion (no related file):

Previously, gordlin (Gordon Lin) wrote…

Latest push fixes a merge conflict. There should be no differences.

Also reduced spacing between the label and inputs by a little bit.

---

[szczz]

Reviewed 1 of 3 files at r2, 1 of 2 files at r5, 2 of 3 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @gordlin and @RyanCoulsonCA)

---

src/components/metadata-editor.vue line 48 at r7 (raw file):

                        >
                        <!-- Button to switch between loading and renaming product UUIDs. -->
                        <span v-if="editExisting && loadStatus == 'loaded'" class="p-1">

===

---

src/components/metadata-editor.vue line 54 at r7 (raw file):

                                    renaming = !renaming;
                                    changeUuid = '';
                                    warning = warning == 'rename' ? 'none' : warning;

===

[gordlin]

Reviewable status: 4 of 5 files reviewed, 5 unresolved discussions (waiting on @RyanCoulsonCA and @szczz)

---

src/components/metadata-editor.vue line 48 at r7 (raw file):

Previously, szczz (Matt Szczerba) wrote…

===

Don't know how this happened so many times. Donethanks!

---

src/components/metadata-editor.vue line 54 at r7 (raw file):

Previously, szczz (Matt Szczerba) wrote…

===

Donethanks!

[RyanCoulsonCA]

Reviewed 1 of 3 files at r2, 2 of 3 files at r6, 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @szczz and @yileifeng)

[IshavSohal]

Looks great!

Reviewed 1 of 3 files at r2, 1 of 2 files at r5, 2 of 3 files at r6, 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @gordlin, @RyanCoulsonCA, @szczz, and @yileifeng)

[szczz]

Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gordlin, @IshavSohal, @RyanCoulsonCA, and @yileifeng)

[spencerwahl]

Reviewed 1 of 3 files at r2, 1 of 2 files at r5, 2 of 3 files at r6, 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gordlin, @RyanCoulsonCA, and @yileifeng)

[yileifeng]

Reviewed 2 of 3 files at r6, 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gordlin, @IshavSohal, @RyanCoulsonCA, @spencerwahl, and @szczz)

[yileifeng]

Reviewed 2 of 2 files at r9, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gordlin, @RyanCoulsonCA, @spencerwahl, and @szczz)