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

enh: remove wrapping element from detachable components #3206

Closed
wants to merge 3 commits into from

Conversation

KaelWD
Copy link
Member

@KaelWD KaelWD commented Feb 7, 2018

Description

Blocked by vuejs/vue#8350, only works with vue <2.5.16

Motivation and Context

fixes #3205, fixes #3107

How Has This Been Tested?

Markup:

<template>
  <v-app>
    <v-container>
      <v-card dark>
        <v-card-text>
          <v-dialog>
            <v-btn slot="activator">dialog 1</v-btn>
            <v-card>
              <v-card-text>
                Dialog 1
              </v-card-text>
            </v-card>
          </v-dialog>
          <v-text-field v-model="text"/>
          <v-dialog>
            <v-btn slot="activator">dialog 2</v-btn>
            <v-card>
              <v-card-text>
                Dialog 2
              </v-card-text>
            </v-card>
          </v-dialog>
        </v-card-text>
      </v-card>
    </v-container>
  </v-app>
</template>

<script>
  export default {
    data: () => ({
      text: ''
    })
  }
</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 feature but make things better)

Checklist:

  • My change requires a change to the documentation.
  • I have created a PR in the documentation with the necessary changes.

@codecov
Copy link

codecov bot commented Feb 7, 2018

Codecov Report

Merging #3206 into dev will increase coverage by 0.29%.
The diff coverage is 67.24%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #3206      +/-   ##
==========================================
+ Coverage   88.89%   89.19%   +0.29%     
==========================================
  Files         227      207      -20     
  Lines        5376     4866     -510     
  Branches     1360     1225     -135     
==========================================
- Hits         4779     4340     -439     
+ Misses        479      420      -59     
+ Partials      118      106      -12
Impacted Files Coverage Δ
src/components/VDialog/VDialogContent.js 64.44% <64.44%> (ø)
src/components/VDialog/VDialog.js 78.57% <76.92%> (+7.14%) ⬆️
src/mixins/dependent.js 54.16% <0%> (-45.84%) ⬇️
src/components/VMenu/mixins/menu-activator.js 44.44% <0%> (-3.71%) ⬇️
src/components/VForm/VForm.js 83.78% <0%> (-3.18%) ⬇️
src/components/VOverflowBtn/VOverflowBtn.js 91.42% <0%> (-3.17%) ⬇️
src/components/VDataTable/VEditDialog.js 20% <0%> (-2.59%) ⬇️
src/mixins/menuable.js 93.13% <0%> (-2.01%) ⬇️
src/mixins/validatable.js 91.8% <0%> (-1.85%) ⬇️
src/mixins/selectable.js 93.87% <0%> (-1.58%) ⬇️
... and 202 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 214e561...bf028a6. Read the comment docs.

@@ -3,6 +3,8 @@ import '../../stylus/components/_bottom-sheets.styl'
import VDialog from '../VDialog/VDialog'

export default {
functional: true,
Copy link

@IPRIT IPRIT Mar 20, 2018

Choose a reason for hiding this comment

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

Do you know that u can't import functional components as Async Component (e.g. VDivider => import('path/to/VDivider.vue')). Server Renderer throw an error because component has no componentOptions and Ctor object.

Copy link
Member Author

Choose a reason for hiding this comment

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

That was fixed in 2.5.16: vuejs/vue#7784

@chewy94 chewy94 added S: in progress and removed S: on hold The issue is on hold until further notice labels Jun 12, 2018
@ghost ghost assigned KaelWD Jun 13, 2018
@ghost ghost added pending team review The issue is pending a full team review and removed S: in progress labels Jun 13, 2018
@KaelWD KaelWD added S: in progress S: on hold The issue is on hold until further notice and removed pending team review The issue is pending a full team review S: in progress labels Jun 13, 2018
functional components can have multiple root nodes, which allows us to
render the dialog side-by-side with its activator
@johnleider
Copy link
Member

Re-open when this is seeing progress again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S: has merge conflicts The pending Pull Request has merge conflicts S: on hold The issue is on hold until further notice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants