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(vuetify): updated layout to be on par with vuetify 2.x #619

Merged
merged 4 commits into from Oct 6, 2020

Conversation

tenzint
Copy link
Contributor

@tenzint tenzint commented Sep 17, 2020

v-row is a wrapper component for v-col. It utilizes flex properties to control the layout and flow of its inner columns. [...] This is the 2.x replacement for v-layout in 1.x.
v-col is a content holder that must be a direct child of v-row. This is the 2.x replacement for v-flex in 1.x.
https://vuetifyjs.com/en/components/grids/#usage

Related: #597

@manniL manniL changed the title fix: updated layout to be on par with vuetify 2.x #597 fix: updated layout to be on par with vuetify 2.x Sep 17, 2020
@manniL
Copy link
Member

manniL commented Sep 17, 2020

LGTM. One leftover is to change the snapshots so the tests are matching.

@tenzint
Copy link
Contributor Author

tenzint commented Sep 18, 2020

hi. I tried but I'm unable to figure out how to start on fixing the snapshots. Can you shine a light on this? I don't have prior experience with PR.

I do know that snapshot is like an image frame of a video.

Should I be doing a git pull to fix this?

@tenzint
Copy link
Contributor Author

tenzint commented Sep 21, 2020

yes. You have to run `jest --updateSnapshot`

@manniL Sorry for the 3 days gap (I had the discord browser closed). I appreciate your help. Since I didn't install jest, I saw that I can run it with npx.
I tried running npx jest --updateSnapshot on all of:
1. the root of create-nuxt-app
2. under packages/create-nuxt-app/test
3. under packages/create-nuxt-app/test/snapshots


I am getting some error messages on all the above directories so I'm guessing I don't know which directory I should be on.
I thought option 3. was the right directory since index.test.js.snap was under there.

@manniL
Copy link
Member

manniL commented Sep 21, 2020

Hey 👋🏻
Running yarn test:snapshot should be enough. You don't need other deps installed besides the ones from create-nuxt-app itself 😋

@tenzint
Copy link
Contributor Author

tenzint commented Sep 22, 2020

With your help I'm able to pass the npm run test:snap locally on my computer.

However, the new commit fails the snapshot check here in the remote.


I checked that I'm on the patch-1 branch with git branch. After running the test:snap successfully, I noticed that index.test.js.snap has updated so I did a git add . on the root and committed it. Then I did git push origin patch-1 and I saw new checks happening on this page.

Sadly, the 2 test failures are still here.


Also, it automatically updated nuxt: '^2.14.5' from version 2.14.4.

@pi0
Copy link
Member

pi0 commented Oct 6, 2020

Hi @tenzint. Thanks for PR. I've rebased with master and updated snapshot 👍

Thanks @manniL for review

@pi0 pi0 changed the title fix: updated layout to be on par with vuetify 2.x fix(vuetify): updated layout to be on par with vuetify 2.x Oct 6, 2020
@pi0 pi0 merged commit 4ceb4c1 into nuxt:master Oct 6, 2020
@tenzint
Copy link
Contributor Author

tenzint commented Oct 6, 2020

Thanks @pi0 and @manniL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants