web tutorials (avpin) #971
Conversation
|
|
averykatko
force-pushed
the
18.0-tutorial-avpin
branch
from
7313c0e to
662c8da
Compare
| slots: { | ||
| type: Object, | ||
| shape: { | ||
| default: true, |
There was a problem hiding this comment.
I think this is useless, can you delete it and see if it changes anything?
There was a problem hiding this comment.
yep, it complains if I remove slots entirely but it's fine without having the shape specified 1771dde
| // shape: { | ||
| // id: String, | ||
| // description: String, | ||
| // }, |
There was a problem hiding this comment.
Don't have commented out code on your commits. If you want to keep history, you already have your commit history.
There was a problem hiding this comment.
Also this looks like something you could use why is it commented out 😆
There was a problem hiding this comment.
this gave me a prop validation error because there were additional fields beyond id and description passed as props so I was gonna look into how to handle that correctly..
There was a problem hiding this comment.
adding "*" into shape makes it work: da104d4
| }; | ||
|
|
||
| setup() { | ||
| console.log("setup dialog"); |
There was a problem hiding this comment.
Remove all console.log and print statements before you commit
There was a problem hiding this comment.
(this would've been caught by pre-commit hooks probably? In real-world PRs if this got in somehow would we want to amend the commit and force-push or just fix it in a subsequent commit?)
There was a problem hiding this comment.
I think the best flow is
- You push a commit and create a PR
- Someone reviews your PR and leaves comments
- You fix the comments with subsequent commits (because at this point if you force push it's difficult for the reviewer to find what they commented on)
- Reviewer approves
- You squash your commits and merge
There was a problem hiding this comment.
But what you said is correct this would be caught by pre-commit
| this.state = useState( | ||
| this.props.items.map( | ||
| item => ({ | ||
| ...item, | ||
| checked: !this.props.excludedItems.includes(item.id), | ||
| }), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Here you're creating a state variable called state and it's value is the items list. A variable called state referring to an item list doesn't make much sense. Maybe you can do something like this.state = useState({itemList: lblalbal});
awesome_dashboard/static/src/dashboard/dashboard_items_dialog/dashboard_items_dialog.xml
Outdated
Show resolved
Hide resolved
awesome_owl/static/src/card/card.js
Outdated
| slots: { | ||
| type: Object, | ||
| shape: { | ||
| default: true, |
There was a problem hiding this comment.
I think this doesn't do anything again. Try removing it
| export class Counter extends Component { | ||
| static template = "awesome_owl.Counter"; | ||
| static props = { | ||
| onChange: {type: Function, required: false}, |
There was a problem hiding this comment.
| onChange: {type: Function, required: false}, | |
| onChange: {type: Function, optional: true}, |
awesome_owl/static/src/main.js
Outdated
| const config = { | ||
| dev: true, | ||
| name: "Owl Tutorial", | ||
| name: "Owl Tutorial" |
There was a problem hiding this comment.
I think this must've been something from how I recovered from losing my commit history in the move from 19.0 to 18.0 base branch by doing things in the wrong order
|
|
||
| # vscode | ||
| .vscode/ |
There was a problem hiding this comment.
Don't push your changes on this file
Co-authored-by: Cemal Faruk Güney <[email protected]>
…pair Co-authored-by: Cemal Faruk Güney <[email protected]>
3 participants
sorry I originally had an actually reasonable commit history for awesome_owl but I messed something up in the switch... oops 🫠