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(grid): don't halve gutters between v-rows #12916

Merged
merged 1 commit into from
Jan 18, 2021

Conversation

KaelWD
Copy link
Member

@KaelWD KaelWD commented Jan 7, 2021

fixes #12915

Markup:

<template>
  <v-container>
    <v-row dense>
      <v-col cols="8">
        <v-card height="100%"></v-card>
      </v-col>
      <v-col cols="4">
        <v-row dense>
          <v-col cols="12">
            <v-card>
              <v-skeleton-loader type="article" />
            </v-card>
          </v-col>
          <v-col cols="12">
            <v-card>
              <v-skeleton-loader type="article" />
            </v-card>
          </v-col>
        </v-row>
      </v-col>
    </v-row>
    <v-row dense>
      <v-col
        v-for="i in 6"
        :key="i"
        cols="6"
      >
        <v-card>
          <v-skeleton-loader type="article" />
        </v-card>
      </v-col>
    </v-row>
  </v-container>
</template>

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

@KaelWD KaelWD added T: bug Functionality that does not work as intended/expected C: VGrid Covers issues involving VContainer, VLayout, VFlex labels Jan 7, 2021
@KaelWD KaelWD added this to the v2.4.x milestone Jan 7, 2021
@KaelWD KaelWD self-assigned this Jan 7, 2021
@Shinigami92
Copy link
Contributor

Any news about this PR?
Is there anything I can do to help get this fix released?

@KaelWD
Copy link
Member Author

KaelWD commented Jan 18, 2021

@vuetifyjs/core-team Nobody reviewed this, if it's broken it's your fault.

@KaelWD KaelWD merged commit f33d0c1 into master Jan 18, 2021
@KaelWD KaelWD deleted the fix/12915-adjacent-row-gutters branch January 18, 2021 17:18
@Shinigami92
Copy link
Contributor

@vuetifyjs/core-team Nobody reviewed this, if it's broken it's your fault.

lol...

Today (18:26 GMT+2) I'm to tired and wont test it
But I will let you know tomorrow after updating and use with a production environment
So you may hear from me in about the next 24 hours 🙂
Good night o/

@Shinigami92
Copy link
Contributor

Shinigami92 commented Jan 18, 2021

... or if it's possible for you, I could also try a beta release 🤔
Like 2.4.3-beta.1


Edit: waiting for the next release
Will then report back to you

@vmihailenco
Copy link

I can confirm that this is broken.

@Shinigami92
Copy link
Contributor

I just arrived at my working station, will test it now

@Shinigami92
Copy link
Contributor

2.3.23

adSoul-Dashboard 2 3 23

2.4.3

adSoul-Dashboard 2 4 3

LGTM

I just removed the pt-0 and pb-0 from my v-cols and don't need any ps or ms in my grid anymore

Just noticed that it moved 4px up, but I think that is intended!


@vmihailenco Could you provide a little bit more information about what is broken for you?
Maybe open a new issue and/or link it here

@vmihailenco
Copy link

vmihailenco commented Jan 22, 2021

@Shinigami92 does this fixes everyting for you "This is just one breaking example and I would have around 80 more files to check with maybe some more complex layout issues."?

This does not fixes the regressions I have in v2.4.x and additionally breaks margin-top, e.g. v-row dense class="mt-4".

@Shinigami92
Copy link
Contributor

Shinigami92 commented Jan 22, 2021

This was just the same example as I showed in #12915
That's why I send it here

But many more sites worked the same for me

Yeah it's a breaking change from 2.3 to 2.4, but this 2.4.3 is the fixed version 2.4 where I think I can finally migrate to 2.4
But I encountered another issue with v-text-field and label+placeholder, but I will search if anyone else has already reported it or open a new one, but this is not related to this issue/pr/fix
=> I found the already opened issue: #12839

I have encountered a part where I have a custom created component where I need to add my-0, but that's totally fine for me because it is a self made component and the "workaround" just worked

image
image
image

Checking more files now
I currently was half way trough my tool :D

@Shinigami92
Copy link
Contributor

Ok, so I think I have visited almost all pages in our tool and it looks a bit more slimmer to me
That's the 4px less at the top (and so I also think at the bottom?)

2.3.23

Screenshot_20210122_091609

2.4.3

Screenshot_20210122_091604


So I can even think about that this change was indeed intended
And so I'm fine with this
Or in other words, this fix fixed more that it broke for me 🙂

@vmihailenco If you encountered other problems, please open a new issue and we can try to work on that
Screenshots of your problems could also help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VGrid Covers issues involving VContainer, VLayout, VFlex T: bug Functionality that does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] Halved gutter width between v-rows
3 participants