-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[IMP] awesome_owl: added counter, card and todo_list components - MOALN #1018
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
base: 18.0
Are you sure you want to change the base?
Conversation
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.
Hi @Mohamed-Khaled308 👋,
Here is a quick review of your PR. My comments are about details. For the git stuff we can check it together tomorrow.
The title of your PR is good, you should just remove your pentagram at the end of it.
Cheers!
| @@ -1,6 +0,0 @@ | |||
| <component name="InspectionProjectProfileManager"> | |||
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.
Can you squash the commit "[REM] tutorials: removed .idea files" inside the commit "[IMP] awesome_owl: added counter and todolist components" ? This will remove all .idea files from your commits, making your PR cleaner.
|
|
||
|
|
||
| export class Card extends Component { | ||
| static template = "awesome_owl.card"; |
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.
| static template = "awesome_owl.card"; | |
| static template = "awesome_owl.Card"; |
| import { Component, useState } from "@odoo/owl"; | ||
|
|
||
| export class Counter extends Component { | ||
| static template = "awesome_owl.counter"; |
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.
Same here. You should use PascalCase for the name after the dot.
|
|
||
|
|
||
| export class TodoItem extends Component { | ||
| static template = "awesome_owl.todo.item"; |
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.
| static template = "awesome_owl.todo.item"; | |
| static template = "awesome_owl.TodoItem"; |
|
|
||
|
|
||
| export class TodoList extends Component { | ||
| static template = "awesome_owl.todo.list"; |
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.
| static template = "awesome_owl.todo.list"; | |
| static template = "awesome_owl.TodoList"; |
| @@ -0,0 +1,14 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <templates xml:space="preserve"> | |||
| <t t-name="awesome_owl.todo.list"> | |||
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.
Same here.
| import { mountComponent } from "@web/env"; | ||
| import { Playground } from "./playground"; | ||
|
|
||
|
|
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.
That diff is not necessary 😃. We try to do as little change as possible as It can generate a big PR and it can make it complicate to read and understand.
| import { Component } from "@odoo/owl"; | ||
|
|
||
| export class Playground extends Component { | ||
| static template = "awesome_owl.playground"; |
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.
They should have used PascalCase 😢. No need to change that.

No description provided.