search

girder / girder_web_components

Reusable Javascript and VueJS components for interacting with a Girder server.
https://gwc.girder.org
Apache License 2.0
16 stars 9 forks source link

Vuetify 2.0 and Vue 2.6 #154

Closed subdavis closed 4 years ago

matthewma7 commented 5 years ago

Is there anything else needed for this PR? Or we are testing it by using it?

subdavis commented 5 years ago

@matthewma7 I don't think there's anything left to do here. However, the code is full of v-layout usage that I haven't converted to v-col and v-row yet because they work very differently.

v-layout appears to have been deprecated silently but still works the same. I'm fine with moving that work elsewhere.

zachmullen commented 5 years ago

because they work very differently.

You mean v-row and v-column are not meant to be a replacement for v-layout(row) and v-layout(column)? What's the appropriate migration path?

matthewma7 commented 5 years ago

v-layout, v-flex to v-row, v-col wasn't super straight forward to me when I was using v-layout vertically. I could give it a try within this PR. And by the way, remove any mentioning of stylus since vuetify 2 switched to scss.

subdavis commented 5 years ago

I believe they're intended that way, but I don't really understand how to replicate some vertical layouts. In particular, fill-height is just gone. Margins are also now a part of the grid directives, which I find myself fighting against every time I use one.

https://vuetifyjs.com/en/getting-started/frequently-asked-questions#how-to-center-vertically

That's the only place in the documentation that v-layout is mentioned. I think it might be a bug.

matthewma7 commented 5 years ago

which I find myself fighting against every time I use one

Exactly!

subdavis commented 4 years ago

@matthewma7 @zachmullen

Where are we here? I'm not exactly happy with the state of things (mostly referring to my own changes) but I think this is holding us all up. I have additional features I need, and cherry picking from a few different branches is getting old.

I see two options:

1) merge it, push it to NPM, and deal with any fallout as it comes up. I don't really have the time to address any major issues that would come up in a code review this week, so we would be merging more or less as-is 2) Start a new beta branch of some kind. This will be a new major or minor version anyway, so we could name it 1.1.0. Merge this to that branch and allow that branch to accept new features such as #155 and #156. Once we feel confident that everything is O.K., maybe in a few weeks, merge to master.

I feel better about option 2, but only on a visceral level.

matthewma7 commented 4 years ago

@subdavis Because Vuetify 1 and 2 can't be used at the same time. This branch is the defacto master branch I am using nowadays. I didn't encounter major issues anymore. So, option 1 would work for me.

subdavis commented 4 years ago

Updated the readme to comply with the new vuetify setup. Added some docs about a-la-carte setup (could @brianhelba take a quick look at that please?)

Once I get @matthewma7 and @zachmullen to sign off, we can merge to master. Thanks especially to @matthewma7 for all your help on this enormous task.

matthewma7 commented 4 years ago

I upgrade the Vuetify to the latest but it broke some unit tests (later fixed). In summary, the unit test issue is caused by the choice of vue-test-util that tries to do synchronized rendering by default, which doesn't work so well anymore.