Skip to content

Conversation

@jvhellemondt
Copy link
Contributor

@jvhellemondt jvhellemondt commented Mar 29, 2021

image

Fixed:

  • Add icon "added"
  • Cannot call class as function -> reverted to how it was.
  • chevron left and right
  • Vuetify resource icons show up as json. Seems to be fixed

Copy link
Contributor

@DennisLammers DennisLammers left a comment

Choose a reason for hiding this comment

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

Ik vind prima, maar wacht maar op @ricardovanlaarhoven :P

@ricardovanlaarhoven
Copy link
Contributor

Sowieso denk ik dat vuetify resource standalone hoort te zijn. Als vuetify niet levert, dan moet de resource zelf iets doen. Doordat we nu SVG gebruiken, zal dat niet veel schelen in de bundle.

i don't agree with that. Vuetify is a peer dependency of the vuetify-resource so you should install that yourself. Just like vue is a peer dependency of vuetify.
The vuetify-resource uses the vuetify icon variables so you can choose your own iconfont and your own icons.

@ricardovanlaarhoven
Copy link
Contributor

chevron left en right

Zitten er in, onder prev / next. Volgens mij worden die gebruikt. Maar als het vuetify resource betreft, dan is dat misschien >anders. Ik heb in vuetify resource gezocht... Maar kon het niet vinden.

Perhaps this was an issue with fontawesome free.

If you've tested it then we can close that issue.
This was about a console warning when using the application in the browser. It said those icons were missing.

@jvhellemondt
Copy link
Contributor Author

Sowieso denk ik dat vuetify resource standalone hoort te zijn. Als vuetify niet levert, dan moet de resource zelf iets doen. Doordat we nu SVG gebruiken, zal dat niet veel schelen in de bundle.

i don't agree with that. Vuetify is a peer dependency of the vuetify-resource so you should install that yourself. Just like vue is a peer dependency of vuetify.
The vuetify-resource uses the vuetify icon variables so you can choose your own iconfont and your own icons.

Vuetify would still be a peer-dependency. And you should be able to overwrite the icons. However, I'd rather have a working vuetify-resource on icons, by default.

Comment on lines +57 to +62
browse to the scaffold-tester folder, which was just created.

install vuetify

`vue add vuetify`

Copy link
Contributor

Choose a reason for hiding this comment

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

there is much more then just vuetify, So should this line be here?

just like above.
my referer to the installation is not really notable, perhaps just change that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is the full instruction to locally test the cli:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

No it isn't. Just like i said before there is much more then vuetify.

vue router
vuex
dart sass
..
..

For development you need to follow the instructions "just like above"
You could add those instructions just like you've added vuetify again at this place. But then you should remove the "just like above"
When you do that we'd have to maintain that at two places so i'm opting to revert your change and just make the line "just like above" more notable.

Something like this:

### Development
- To test/develop the kingscode scaffold you can locally invoke this plugin.
create a test project (`vue create scaffold-tester`) just like in the Installation part, you can follow those instructions and continue here.

- install the cli-plugin locally

`npm install path\to\local\vue-cli-plugin-kingscode-scaffold`

- invoke the plugin

`vue invoke kingscode-scaffold`

Copy link
Contributor Author

@jvhellemondt jvhellemondt Mar 31, 2021

Choose a reason for hiding this comment

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

I'm not sure why you keep pushing. The only fact is, my icon changes made Vuetify the only one mandatory for this cli (according to the instructions). Vue router, vuex and even dart sass (the dart sass thing is not required, if you don't select css pre-processors), are optional (according to the instructions. I did no changes in that). Thus without 'adding' vuetify, the development instruction is incomplete.

I do not mind changing the 'just like above' part. However, I think it's still valid.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's call about this.
BTW PR was already approved so i'm not pushing but in my opinion this adds more confusion instead of removing it.

@jvhellemondt jvhellemondt merged commit 4956b5d into main Mar 31, 2021
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.

5 participants