-
Notifications
You must be signed in to change notification settings - Fork 4
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 SinglePageLayout Vue warnings #11
Conversation
30 |
Did you run the standard test suite? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks excellent. Thank you for this.
9/10/10
I believe this repository has an automated testing workflow that will be run. |
Excellent. Approved! Thanks for contributing to this great project |
I would not change those since they are directives but for beginners it is good to be aware of it. |
Do you have a suggestion on another way to fix the warnings? |
We could remove the warning on the directive. But those are only display when you run with |
I thought the warnings were indicating that you should be using a tuple? Every time we find these warnings, we use this solution to resolve them. |
The idea is to be explicit and coherent with the trame syntax. But as long as you trully understand how directive works, then skipping the tuple give you a more compact syntax. Also, providing the default value tends to force you to use a tuple in your code, which is indeed good. But for that "fixed" template, there is no point to provide a default. |
but since it is at the layout level, to prevent confusion, I think I would be ok to merge that. I'm just saying that if that was my code in an app, I would not want to change it. ;-) |
Fixes the following warnings: