Skip to content

Follow-up from "Resolve "Provide optional title and description before submitting edits made with the Static Site Editor""

The following discussion from !44512 (merged) should be addressed:

  • @ealcantara started a discussion:

    suggestion (non blocking): This line caught my attention because it made me ask: Why does "edit_meta_modal" has to reset the local storage when edit_meta_controls is the component in charge of setting/removing state from that source? Then I noticed that the state we are storing in the local storage is precisely mergeRequestMeta and I also noticed that we have a editable object in edit_meta_controls that is just a copy of mergeRequestMeta.

    I think we can simplify significantly the state management in these two components by using mergeRequestMeta as a single source of truth for state. It also makes sense to save and restore mergeRequestMeta from this component to keep state management as self-contained as possible. This is a patch that demonstrates it:

    diff --git a/app/assets/javascripts/static_site_editor/components/edit_meta_controls.vue b/app/assets/javascripts/static_site_editor/components/edit_meta_controls.vue
    index 9f75c65a316..fb80b158b46 100644
    --- a/app/assets/javascripts/static_site_editor/components/edit_meta_controls.vue
    +++ b/app/assets/javascripts/static_site_editor/components/edit_meta_controls.vue
    @@ -1,6 +1,5 @@
     <script>
     import { GlForm, GlFormGroup, GlFormInput, GlFormTextarea } from '@gitlab/ui';
    -import AccessorUtilities from '~/lib/utils/accessor';
     
     export default {
       components: {
    @@ -19,55 +18,25 @@ export default {
           required: true,
         },
       },
    -  data() {
    -    return {
    -      editable: {
    -        title: this.title,
    -        description: this.description,
    -      },
    -    };
    -  },
    -  computed: {
    -    editableStorageKey() {
    -      return this.getId('local-storage', 'editable');
    -    },
    -    hasLocalStorage() {
    -      return AccessorUtilities.isLocalStorageAccessSafe();
    -    },
    -  },
       mounted() {
    -    this.initCachedEditable();
         this.preSelect();
       },
       methods: {
         getId(type, key) {
           return `sse-merge-request-meta-${type}-${key}`;
         },
    -    initCachedEditable() {
    -      if (this.hasLocalStorage) {
    -        const cachedEditable = JSON.parse(localStorage.getItem(this.editableStorageKey));
    -        if (cachedEditable) {
    -          this.editable = cachedEditable;
    -        }
    -      }
    -    },
         preSelect() {
           this.$nextTick(() => {
             this.$refs.title.$el.select();
           });
         },
    -    resetCachedEditable() {
    -      if (this.hasLocalStorage) {
    -        window.localStorage.removeItem(this.editableStorageKey);
    -      }
    -    },
    -    onUpdate() {
    -      const payload = { ...this.editable };
    +    onUpdate(field, value) {
    +      const payload = {
    +        title: this.title,
    +        description: this.description,
    +        [field]: value,
    +      };
           this.$emit('updateSettings', payload);
    -
    -      if (this.hasLocalStorage) {
    -        window.localStorage.setItem(this.editableStorageKey, JSON.stringify(payload));
    -      }
         },
       },
     };
    @@ -83,9 +52,9 @@ export default {
           <gl-form-input
             :id="getId('control', 'title')"
             ref="title"
    -        v-model.lazy="editable.title"
    +        :value="title"
             type="text"
    -        @input="onUpdate"
    +        @input="onUpdate('title', $event)"
           />
         </gl-form-group>
     
    @@ -96,8 +65,8 @@ export default {
         >
           <gl-form-textarea
             :id="getId('control', 'description')"
    -        v-model.lazy="editable.description"
    -        @input="onUpdate"
    +        :value="description"
    +        @input="onUpdate('description', $event)"
           />
         </gl-form-group>
       </gl-form>
    diff --git a/app/assets/javascripts/static_site_editor/components/edit_meta_modal.vue b/app/assets/javascripts/static_site_editor/components/edit_meta_modal.vue
    index 4e5245bd892..468acd21a3e 100644
    --- a/app/assets/javascripts/static_site_editor/components/edit_meta_modal.vue
    +++ b/app/assets/javascripts/static_site_editor/components/edit_meta_modal.vue
    @@ -1,9 +1,12 @@
     <script>
     import { GlModal } from '@gitlab/ui';
     import { __, s__, sprintf } from '~/locale';
    +import AccessorUtilities from '~/lib/utils/accessor';
     
     import EditMetaControls from './edit_meta_controls.vue';
     
    +const LOCAL_STORAGE_KEY = 'sse-merge-request-meta-storage-key';
    +
     export default {
       components: {
         GlModal,
    @@ -29,6 +32,9 @@ export default {
         disabled() {
           return this.mergeRequestMeta.title === '';
         },
    +    hasLocalStorage() {
    +      return AccessorUtilities.isLocalStorageAccessSafe();
    +    },
         primaryProps() {
           return {
             text: __('Submit changes'),
    @@ -42,6 +48,10 @@ export default {
           };
         },
       },
    +
    +  mounted() {
    +    this.initCachedEditable();
    +  },
       methods: {
         hide() {
           this.$refs.modal.hide();
    @@ -51,13 +61,28 @@ export default {
         },
         onPrimary() {
           this.$emit('primary', this.mergeRequestMeta);
    -      this.$refs.editMetaControls.resetCachedEditable();
    +
    +      if (this.hasLocalStorage) {
    +        window.localStorage.removeItem(LOCAL_STORAGE_KEY);
    +      }
         },
         onSecondary() {
           this.hide();
         },
         onUpdateSettings(mergeRequestMeta) {
           this.mergeRequestMeta = { ...mergeRequestMeta };
    +
    +      if (this.hasLocalStorage) {
    +        window.localStorage.setItem(LOCAL_STORAGE_KEY, JSON.stringify(mergeRequestMeta));
    +      }
    +    },
    +    initCachedEditable() {
    +      if (this.hasLocalStorage) {
    +        const cachedEditable = JSON.parse(localStorage.getItem(LOCAL_STORAGE_KEY));
    +        if (cachedEditable) {
    +          this.mergeRequestMeta = cachedEditable;
    +        }
    +      }
         },
       },
     };

    A related observation regarding the local storage key is that we could keep it as a constant in constants.js instead of making it a reactive, computed property. This suggestion is non-blocking because this structure is very self-contained and it won’t affect anything else outside this feature.