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

Merging work on new UI into master branch #1461

Closed
wants to merge 8 commits into from

Conversation

zklosko
Copy link
Member

@zklosko zklosko commented Dec 23, 2021

Description

Hey everyone, this is the work I've been doing to extend @hairmare's starter Vue UI. This PR completes #1460.

Notes

The new UI is not stable but does not conflict with the rest of the repo.

Links

See #1122 for initial planning.

Copy link
Contributor

@jooola jooola left a comment

Choose a reason for hiding this comment

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

In addition to the many comments I made, here is list of things I would like to address before this is merged.

  • The PNG icons should be replaced with SVG from a icons package.
  • Allow to use typescript files.
  • Use Vue defineComponent function for type hinting, this should help when using an IDE and reduce dumb errors.
  • Use Vue3.
  • Use Composition API.

This PR ships a lot of visual code, but not yet functional. This is good to start shaping the future UI, but I think we should focus on building blocks after blocks in coordination with the django API.

I was actually hoping that Libretime consider React instead of Vue. And I would be in favor of Boostrap instead of Vuetify. But yeah, I don't know if this will be reconsidered.

Anyway nice work @zklosko

ui/jest.config.js Show resolved Hide resolved
"axios": "^0.21.1",
"core-js": "^3.8.1",
"dayjs": "^1.10.7",
"howler": "^2.2.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need howler ? Directly using the Web Audio API is probably enough ?

"howler": "^2.2.3",
"v-calendar": "^2.1.4",
"vue": "^2.6.11",
"vue-blob-json-csv": "^0.1.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this used for ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it might be part of the prototyping? I'm honestly not sure.

Comment on lines +36 to +41
"@vue/cli-plugin-babel": "~4.5.0",
"@vue/cli-plugin-eslint": "~4.5.0",
"@vue/cli-plugin-router": "~4.5.0",
"@vue/cli-plugin-unit-jest": "~4.5.0",
"@vue/cli-service": "^4.5.0",
"@vue/test-utils": "^1.1.2",
Copy link
Contributor

@jooola jooola Dec 23, 2021

Choose a reason for hiding this comment

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

I thought you wanted to use Vitejs ? I think Vite isn't a bad idea. We should check it won't be a blocker for some tools integrations, such as Jest/Cypress/..., but I don't think so.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would be a good idea. The scaffolding was done by @hairmare; I guess he was more comfortable with Vue CLI (which ships Webpack) so that's what we're currently running with. If we are to start again with Vue3 it might be easier to scaffold with Vite.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've been trying to integrate everything with Vite and it just isn't working. Even using templates from Github haven't gotten me anywhere, especially since Vuetify 3 is still in alpha and relatively unstable. Webpack isn't a deal-breaker but it's crazy slow, especially as we develop.

Copy link
Contributor

Choose a reason for hiding this comment

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

And what with this ? vitejs/vite#305

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly, all of this stuff is over my head. Again, why I'm trying to open it up for more people to work on.

Comment on lines +53 to +54
"prism-react-renderer": "^1.1.1",
"prismjs": "^1.17.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not a clue. It might be a dependent for Storyblocks or something.

ui/src/assets/styles/base.sass Outdated Show resolved Hide resolved
ui/src/components/App/AppBar.vue Show resolved Hide resolved
</script>

<style lang="sass">
@use '../../assets/styles/base'
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this line in many components, what about importing the global styles in the app directly (without scope) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Trying, but without success

Copy link
Contributor

@jooola jooola Dec 28, 2021

Choose a reason for hiding this comment

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

You should be able to import the global .scss file into main.js or App.vue like this:

import "./styles/globals.sass"

It should then be picked up by the building tool and added to the html entrypoint head.

<v-form>
<v-row>
<v-col>
<v-text-field v-model="userInfo.first_name" label="First Name"></v-text-field>
Copy link
Contributor

Choose a reason for hiding this comment

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

We should enforce same casing style everywhere.

ui/src/locales/en-US.json Show resolved Hide resolved
@jooola
Copy link
Contributor

jooola commented Dec 23, 2021

Ohh and if you able to keep the commit history it would be awesome. Hairmaire managed to have a nice commit history of the changes, I hope we can do this here as well.

@zklosko
Copy link
Member Author

zklosko commented Dec 24, 2021

Ohh and if you able to keep the commit history it would be awesome. Hairmaire managed to have a nice commit history of the changes, I hope we can do this here as well.

I wasn't able to move commits over from his branch to my branch.

I was actually hoping that Libretime consider React instead of Vue. And I would be in favor of Boostrap instead of Vuetify. But yeah, I don't know if this will be reconsidered.

I kind of agree. All I know is that we talked about it and @Haremare was leading the charge. Now he's stepped back (temporarily?) and development is slow. I don't think it's too late to turn back, especially in favor of meta-frameworks like Next.js or Nuxt.js that make development faster. It's going to be rather difficult to switch design frameworks and make the new UI similar to the old UI.

@paddatrapper unless I'm mistaken, the only objectives that are firmly set are

  1. Decoupled front-end from back-end, with Node.js and Python, respectively
  2. Similar UI design to PHP MVC so it's easy for users to transition

so it sounds like we can still talk out everything before we have a firmer code foundation.

@paddatrapper
Copy link
Contributor

We can't use Vue3, as Vuetify doesn't support it

@jooola
Copy link
Contributor

jooola commented Dec 25, 2021

If we really want to stick to to vue2 we should then use https://github.com/vuejs/composition-api

@zklosko
Copy link
Member Author

zklosko commented Dec 26, 2021

Vuetify 3 (which does support Vue 3) is set to release in Feburary 2022 and does have first party support for Vite. It might benefit us to wait it out before we begin development, making sure the API is fully ready and getting a beta 3.0 out with PHP.

@paddatrapper
Copy link
Contributor

I'm happy with that, though if we use the composition-api plugin, how much migration will be required between Vue 2 and 3?

@jooola
Copy link
Contributor

jooola commented Dec 28, 2021

Right now I don't see the real requirement on Vuetify, we could simply start without it. Our main goal is the write small pieces that can be embed directly in the legacy ui. So I would push for starting with vue3 directly, and introduce a UI framework only later (so we leave time for both boostrap-vue and vuetify to support vue3).

Using the composition api plugin is to use the power of this feature without vue3, and allow a easy migration to vue3, as it would simply mean, replacing the vue composition plugin import by vue (from there README: When you migrate to Vue 3, just replacing @vue/composition-api to vue and your code should just work.).

From what I see in this PR, I am starting to think that we might want to consider merging this in a separate branch, and pick code to implement working views in separate PR's. I was hoping to build one view after the other, and not push everything at once while nothing is fully working. Having this in a separate branch might be a good idea.

One of the first features I would like to see implemented is the Settings view or Storage / Upload view. Once this is build and embed into the legacy app we will have a better idea of what is working and what needs improvements.

@paddatrapper
Copy link
Contributor

I'm going to need some time in the new year to review this. Hopefully just the first week. In the mean time - this cannot go into a feature branch, as the C4 explicitly forbids feature branches in the project repo:

2.5.1 The project SHALL have one branch (“master”) that always holds the latest in-progress version and SHOULD always build.

2.5.2 The project SHALL NOT use topic branches for any reason. Personal forks MAY use topic branches.

@zklosko
Copy link
Member Author

zklosko commented Dec 28, 2021

We need a UI framework just to get started. Vuetify gives us interactive elements we can use out of the box with minimal configuration - like upload dialogues, calendars, and datatables - that other frameworks like Bootstrap don't provide.

For the PHP UI, we had three separate dependencies for the three features I mentioned above. It's easier to have a single import.

@jooola
Copy link
Contributor

jooola commented Jan 3, 2022

According to the Vuetify roadmap, vue3 support is expected to be beta in December 2021 and "stable" on February 2022 https://vuetifyjs.com/en/introduction/roadmap/#v3-0-titan

They already have some alpha release, beta release might not be that far away.

So we might still want to consider vue3. Haha sorry for pushing hard on this one, I'll try to provide some working code with vue3 instead of just complaining.

@jooola
Copy link
Contributor

jooola commented Jan 6, 2022

Some poc work with vue3 https://github.com/jooola/libretime/tree/feat/web_ui/ui

@zklosko zklosko marked this pull request as draft January 28, 2022 15:33
@zklosko
Copy link
Member Author

zklosko commented Feb 6, 2022

Update from the Vuetify team: they're now expecting Vuetify 3 to be released around May 2022.

zklosko added a commit to zklosko/libretime that referenced this pull request Mar 26, 2022
@zklosko
Copy link
Member Author

zklosko commented Mar 26, 2022

Vuetify 3 beta has officially been released! On top of that, the new Vue CLI installs and configures Vite, which makes our goals so much easier. I'm starting over with Vue 3, Vuetify 3 beta, Vite, Typescript, and the composition API; I'll create the PR soon.

Closing this PR for clarity.

@zklosko zklosko closed this Mar 26, 2022
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