Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bootable): don't generate lazy content until it's rendered #8823

Merged
merged 14 commits into from
Jan 31, 2020

Conversation

KaelWD
Copy link
Member

@KaelWD KaelWD commented Sep 1, 2019

Motivation and Context

fixes #8712

better performance, fewer empty DOM elements, skips a bunch of calculations for hidden elements

Markup:

<template>
  <v-container>
    <v-dialog
      v-model="dialog"
      width="500"
    >
      <template v-slot:activator="{ on }">
        <v-btn
          color="red lighten-2"
          dark
          v-on="on"
        >
          Dialog
        </v-btn>
      </template>

      <v-card>
        <v-card-title
          class="headline grey lighten-2"
          primary-title
        >
          Privacy Policy
        </v-card-title>

        <v-card-text>
          Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
        </v-card-text>

        <v-divider></v-divider>

        <v-card-actions>
          <v-spacer></v-spacer>
          <v-btn
            color="primary"
            text
            @click="dialog = false"
          >
            I accept
          </v-btn>
        </v-card-actions>
      </v-card>
    </v-dialog>

    <v-tooltip bottom>
      <template v-slot:activator="{ on }">
        <v-btn color="primary" dark v-on="on">Tooltip</v-btn>
      </template>
      <span>Tooltip</span>
    </v-tooltip>

    <v-menu offset-y>
      <template v-slot:activator="{ on }">
        <v-btn
          color="primary"
          dark
          v-on="on"
        >
          Menu
        </v-btn>
      </template>
      <v-list>
        <v-list-item
          v-for="(item, index) in items"
          :key="index"
          @click=""
        >
          <v-list-item-title>{{ item.title }}</v-list-item-title>
        </v-list-item>
      </v-list>
    </v-menu>

    <v-expansion-panels>
      <v-expansion-panel
        v-for="(item,i) in 5"
        :key="i"
      >
        <v-expansion-panel-header>Item</v-expansion-panel-header>
        <v-expansion-panel-content>
          Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
        </v-expansion-panel-content>
      </v-expansion-panel>
    </v-expansion-panels>

    <v-card
      max-width="500"
      class="mx-auto"
    >
      <v-toolbar
        color="teal"
        dark
      >
        <v-app-bar-nav-icon></v-app-bar-nav-icon>

        <v-toolbar-title>Topics</v-toolbar-title>

        <v-spacer></v-spacer>

        <v-btn icon>
          <v-icon>mdi-dots-vertical</v-icon>
        </v-btn>
      </v-toolbar>

      <v-list>
        <v-list-group
          v-for="item in items"
          :key="item.title"
          v-model="item.active"
          :prepend-icon="item.action"
          no-action
        >
          <template v-slot:activator>
            <v-list-item-content>
              <v-list-item-title v-text="item.title"></v-list-item-title>
            </v-list-item-content>
          </template>

          <v-list-item
            v-for="subItem in item.items"
            :key="subItem.title"
            @click=""
          >
            <v-list-item-content>
              <v-list-item-title v-text="subItem.title"></v-list-item-title>
            </v-list-item-content>
          </v-list-item>
        </v-list-group>
      </v-list>
    </v-card>

    <v-carousel
      cycle
      height="400"
      hide-delimiter-background
      show-arrows-on-hover
    >
      <v-carousel-item
        v-for="(slide, i) in slides"
        :key="i"
      >
        <v-sheet
          :color="colors[i]"
          height="100%"
        >
          <v-row
            class="fill-height"
            align="center"
            justify="center"
          >
            <div class="display-3">{{ slide }} Slide</div>
          </v-row>
        </v-sheet>
      </v-carousel-item>
    </v-carousel>

    <v-card flat tile>
      <v-window v-model="onboarding" vertical>
        <v-window-item
          v-for="n in length"
          :key="`card-${n}`"
        >
          <v-card
            color="grey"
            height="200"
          >
            <v-row
              class="fill-height"
              align="center"
              justify="center"
              tag="v-card-text"
            >
              <h1 style="font-size: 5rem;" class="white--text">Slide {{ n }}</h1>
            </v-row>
          </v-card>
        </v-window-item>
      </v-window>

      <v-card-actions class="justify-space-between">
        <v-btn
          text
          @click="prev"
        >
          <v-icon>mdi-chevron-left</v-icon>
        </v-btn>
        <v-item-group
          v-model="onboarding"
          class="text-center"
          mandatory
        >
          <v-item
            v-for="n in length"
            :key="`btn-${n}`"
            v-slot:default="{ active, toggle }"
          >
            <v-btn
              :input-value="active"
              icon
              @click="toggle"
            >
              <v-icon>mdi-record</v-icon>
            </v-btn>
          </v-item>
        </v-item-group>
        <v-btn
          text
          @click="next"
        >
          <v-icon>mdi-chevron-right</v-icon>
        </v-btn>
      </v-card-actions>
    </v-card>
  </v-container>
</template>

<script>
  export default {
    data: () => ({
      dialog: false,

      items: [
        {
          action: 'local_activity',
          title: 'Attractions',
          items: [
            { title: 'List Item' },
          ],
        },
        {
          action: 'restaurant',
          title: 'Dining',
          active: true,
          items: [
            { title: 'Breakfast & brunch' },
            { title: 'New American' },
            { title: 'Sushi' },
          ],
        },
        {
          action: 'school',
          title: 'Education',
          items: [
            { title: 'List Item' },
          ],
        },
        {
          action: 'directions_run',
          title: 'Family',
          items: [
            { title: 'List Item' },
          ],
        },
        {
          action: 'healing',
          title: 'Health',
          items: [
            { title: 'List Item' },
          ],
        },
        {
          action: 'content_cut',
          title: 'Office',
          items: [
            { title: 'List Item' },
          ],
        },
        {
          action: 'local_offer',
          title: 'Promotions',
          items: [
            { title: 'List Item' },
          ],
        },
      ],

      colors: [
        'indigo',
        'warning',
        'pink darken-2',
        'red lighten-1',
        'deep-purple accent-4',
      ],
      slides: [
        'First',
        'Second',
        'Third',
        'Fourth',
        'Fifth',
      ],

      length: 3,
      onboarding: 0,
    }),
    methods: {
      next () {
        this.onboarding = this.onboarding + 1 === this.length
          ? 0
          : this.onboarding + 1
      },
      prev () {
        this.onboarding = this.onboarding - 1 < 0
          ? this.length - 1
          : this.onboarding - 1
      },
    },
  }
</script>

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any features but makes things better)

@KaelWD KaelWD added the performance The issue involves performance label Sep 1, 2019
@KaelWD KaelWD self-assigned this Sep 1, 2019
@TravisBuddy
Copy link

TravisBuddy commented Sep 1, 2019

Hey @KaelWD,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 4a236d60-4264-11ea-aec0-5f1345ae5aee

@ascott18
Copy link
Contributor

ascott18 commented Sep 1, 2019

Wouldn't it be best to use a direct if (this.hasContent) { ... } check instead of showLazyContent? Because unless I'm mistaken, with showLazyContent the whole VNode tree still gets created in this case (but just not patched into the DOM because showLazyContent will just throw it away.)

@KaelWD
Copy link
Member Author

KaelWD commented Sep 2, 2019

Ah right good catch

Copy link
Member

@johnleider johnleider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Add unit tests please.

@johnleider johnleider added the S: has merge conflicts The pending Pull Request has merge conflicts label Sep 24, 2019
@ascott18
Copy link
Contributor

ascott18 commented Oct 2, 2019

Would love to see this get wrapped up if you guys can find the time! Especially the change to replace the usage showLazyContent with direct branching (or change showLazyContent to accept a function that will lazily create the VNode tree only if needed).

I understand that you guys are busy with all sorts of other good stuff, though, and I appreciate all the work that you do!

@johnleider
Copy link
Member

Pulse check.

@johnleider johnleider closed this Oct 11, 2019
@johnleider
Copy link
Member

This Pull Request is being closed due to inactivity.

@lock lock bot locked as resolved and limited conversation to collaborators Nov 10, 2019
@KaelWD KaelWD reopened this Jan 13, 2020
@vuetifyjs vuetifyjs unlocked this conversation Jan 13, 2020
@KaelWD
Copy link
Member Author

KaelWD commented Jan 13, 2020

Related: #10201

@KaelWD KaelWD added S: work in progress and removed S: has merge conflicts The pending Pull Request has merge conflicts labels Jan 13, 2020
@KaelWD KaelWD requested review from johnleider and a team January 15, 2020 08:24
@KaelWD KaelWD changed the title fix: avoid creating ThemeProvider in lazy dialogs and menus fix(bootable): don't generate lazy content until it's rendered Jan 15, 2020
@KaelWD KaelWD added C: VCarousel VCarousel C: VDialog VDialog labels Jan 15, 2020
@KaelWD KaelWD added C: VExpansionPanels VExpansionPanels C: VListGroup VListGroup C: VMenu VMenu C: VTooltip VTooltip C: VWindow VWindow labels Jan 15, 2020
@KaelWD KaelWD requested a review from johnleider January 29, 2020 08:35
@KaelWD KaelWD merged commit 2b975f5 into master Jan 31, 2020
@KaelWD KaelWD deleted the fix/8712-lazy-ThemeProvider branch January 31, 2020 07:00
KaelWD added a commit that referenced this pull request Jan 31, 2020
KaelWD added a commit that referenced this pull request Jan 31, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: VCarousel VCarousel C: VDialog VDialog C: VExpansionPanels VExpansionPanels C: VListGroup VListGroup C: VMenu VMenu C: VTooltip VTooltip C: VWindow VWindow performance The issue involves performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] [VDialog] [Performance] Don't create ThemeProvider when dialog is lazy and not visible
5 participants