Skip to content
Snippets Groups Projects

Wrapper for gl-tab to lazily load contents once

Merged Miguel Rincon requested to merge 285473-wrapper-mounted-tab into master

What does this MR do?

Proposed solution for #285473

Background

Two tabs in our editor require rendering large amounts of DOM elements: The editor tab, which depends on EditorLite and the visualization tab, which renders the pipeline graph.

They both calculate their size depending on the wrapping component, which can be problematic when using tabs: by default, hidden tabs are mounted along with their contents while hidden, which prevents correctly calculating the size of a container.

Previous Solution

Use gl-tab's lazy prop to render the component only when it is shown. This works in some cases, but it also dismounts the contents when the tab is not visible. This is undesirable as components like the editor should only be hidden and not fully dismounted every time they are hidden.

We used lazy in some creative ways to change it's behavior, but this is proving confusing.

Proposed solution

Define a new wrapper for the gl-tab component that is lazy and stays mounted after being hidden.

Screenshots (strongly suggested)

As a test I added a "fake" tab, to see when the other two components in the tab would get created() and mounted():

diff of test (slightly out of date, `editor-lazy-tab` was renamed to `editor-tab`)
diff --git a/app/assets/javascripts/pipeline_editor/components/text_editor.vue b/app/assets/javascripts/pipeline_editor/components/text_editor.vue
index 22f2a32c9ac..c6d96f2d445 100644
--- a/app/assets/javascripts/pipeline_editor/components/text_editor.vue
+++ b/app/assets/javascripts/pipeline_editor/components/text_editor.vue
@@ -5,6 +5,12 @@ export default {
   components: {
     EditorLite,
   },
+  created() {
+    console.log('text-editor created()');
+  },
+  mounted() {
+    console.log('text-editor mounted()');
+  },
 };
 </script>
 <template>
diff --git a/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue b/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue
index c9460a3fe5a..a7c9c345910 100644
--- a/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue
+++ b/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue
@@ -251,6 +251,16 @@ export default {
       <gl-loading-icon v-if="isBlobContentLoading" size="lg" class="gl-m-3" />
       <div v-else class="file-editor gl-mb-3">
         <gl-tabs>
+          <editor-lazy-tab :title="'Empty for testing'">
+            <pre>
+
+
+              Empty contents
+
+
+            </pre>
+          </editor-lazy-tab>
+
           <editor-lazy-tab :title="$options.i18n.tabEdit">
             <text-editor v-model="contentModel" />
           </editor-lazy-tab>
diff --git a/app/assets/javascripts/pipelines/components/pipeline_graph/pipeline_graph.vue b/app/assets/javascripts/pipelines/components/pipeline_graph/pipeline_graph.vue
index 11ad2f2a3b6..93a146a7654 100644
--- a/app/assets/javascripts/pipelines/components/pipeline_graph/pipeline_graph.vue
+++ b/app/assets/javascripts/pipelines/components/pipeline_graph/pipeline_graph.vue
@@ -79,7 +79,11 @@ export default {
       return [];
     },
   },
+  created() {
+    console.log('pipeline-graph created()');
+  },
   mounted() {
+    console.log('pipeline-graph mounted()');
     if (!this.isPipelineDataEmpty) {
       this.getGraphDimensions();
       this.drawJobLinks();

2020-12-10_12.48.59

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Miguel Rincon

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Bundle size analysis [beta]

    This compares changes in bundle size for entry points between the commits 457228c4 and 479abcc3

    :sparkles: Special assets

    Entrypoint / Name Size before Size after Diff Diff in percent
    average 3.04 MB 3.04 MB - 0.0 %
    mainChunk 1.89 MB 1.89 MB - 0.0 %

    Please look at the full report for more details


    Read more about how this report works.

    Generated by :no_entry_sign: Danger

    Edited by 🤖 GitLab Bot 🤖
  • removed auto updated label

  • Miguel Rincon marked this merge request as draft

    marked this merge request as draft

  • Miguel Rincon changed the description

    changed the description

  • Miguel Rincon added 1 commit

    added 1 commit

    • aba16637 - Wrapper for gl-tab to lazily load contents once

    Compare with previous version

  • Miguel Rincon mentioned in issue #285473

    mentioned in issue #285473

  • Miguel Rincon changed title from Draft: wrapper for gl-tab to lazily load contents once to Draft: Wrapper for gl-tab to lazily load contents once

    changed title from Draft: wrapper for gl-tab to lazily load contents once to Draft: Wrapper for gl-tab to lazily load contents once

  • assigned to @mrincon

  • Miguel Rincon added 1202 commits

    added 1202 commits

    Compare with previous version

  • Miguel Rincon marked this merge request as ready

    marked this merge request as ready

  • Miguel Rincon changed the description

    changed the description

  • Nicolò Maria Mezzopera
  • Miguel Rincon assigned to @farias-gl and unassigned @f_caplette

    assigned to @farias-gl and unassigned @f_caplette

  • Miguel Rincon added 1 commit

    added 1 commit

    • 75876a09 - Add tab wrapper to lazily mount content only once

    Compare with previous version

  • Miguel Rincon mentioned in merge request !48901 (merged)

    mentioned in merge request !48901 (merged)

  • mentioned in issue #295203 (closed)

  • -
    • Contributor
      Resolved by Nicolò Maria Mezzopera

      @mrincon the feedback I left back doesn't really request you to change anything. It more or captures what I discovered in the process of trying to understand this MR.

      My only suggestion is to create a follow-up issue just to track the possibility of moving it to gitlab-ui at a later date. I think we are ok for now. The moment we use the component in two+ places I think it might benefit to move to gitlab-ui.

      Great work!

  • - assigned to @mrincon

    assigned to @mrincon

  • unassigned @farias-gl

  • Miguel Rincon mentioned in issue #295272

    mentioned in issue #295272

  • Miguel Rincon added 340 commits

    added 340 commits

    Compare with previous version

  • Miguel Rincon added 1 commit

    added 1 commit

    • eff8130f - Add tab wrapper to lazily mount content only once

    Compare with previous version

  • Miguel Rincon assigned to @farias-gl and unassigned @mrincon

    assigned to @farias-gl and unassigned @mrincon

  • Miguel Rincon added 21 commits

    added 21 commits

    Compare with previous version

  • Miguel Rincon added 16 commits

    added 16 commits

    Compare with previous version

  • Nicolò Maria Mezzopera approved this merge request

    approved this merge request

  • - approved this merge request

    approved this merge request

  • unassigned @farias-gl

  • - requested review from @nmezzopera

    requested review from @nmezzopera

  • Miguel Rincon added 102 commits

    added 102 commits

    Compare with previous version

  • Miguel Rincon changed milestone to %13.8

    changed milestone to %13.8

  • Miguel Rincon changed the description

    changed the description

  • Miguel Rincon marked the checklist item Code review guidelines as completed

    marked the checklist item Code review guidelines as completed

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading