Skip to content

[IMP] awesome_owl: add counter component as sub component to playground #747

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

Open
wants to merge 13 commits into
base: 18.0
Choose a base branch
from

Conversation

msho-odoo
Copy link

No description provided.

@robodoo
Copy link

robodoo commented Apr 29, 2025

Pull request status dashboard

@SergeBayet
Copy link

@msho-odoo
Just passing by, don't forget the blank line at the end of your files. And remove the header odoo-module at the top of the js files. This is not necessary anymore. Cheers.

@msho-odoo msho-odoo force-pushed the 18.0-owl-tutorial-msho branch from 0a27663 to 1d61c05 Compare May 2, 2025 07:32
@SergeBayet
Copy link

@msho-odoo Just passing by, don't forget the blank line at the end of your files. And remove the header odoo-module at the top of the js files. This is not necessary anymore. Cheers.

And for the XML files as well :)

@msho-odoo msho-odoo force-pushed the 18.0-owl-tutorial-msho branch from 108f182 to c387d41 Compare May 2, 2025 09:24
@@ -2,6 +2,7 @@
<templates xml:space="preserve">

<t t-name="awesome_owl.todoitem">
<input type="checkbox" t-on-change="()=> props.onToggleState(props.todo.id)"/>

Choose a reason for hiding this comment

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

Suggested change
<input type="checkbox" t-on-change="()=> props.onToggleState(props.todo.id)"/>
<input type="checkbox" t-on-change="() => props.onToggleState(props.todo.id)"/>

@@ -23,4 +23,9 @@ export class TodoList extends Component {
ev.target.value = "";
}
};

toggleState(todoId) {
let todo = this.state.todos.find(todo => todo.id === todoId);

Choose a reason for hiding this comment

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

Suggested change
let todo = this.state.todos.find(todo => todo.id === todoId);
const todo = this.state.todos.find(todo => todo.id === todoId);

@@ -7,7 +7,7 @@
<t t-foreach="state.todos" t-as="i" t-key="i.id">

Choose a reason for hiding this comment

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

Convention: foreach -> plural, t-as -> singular.

Suggested change
<t t-foreach="state.todos" t-as="i" t-key="i.id">
<t t-foreach="state.todos" t-as="todo" t-key="todo.id">

@@ -7,7 +7,7 @@
<t t-foreach="state.todos" t-as="i" t-key="i.id">
<div style="border: 1px solid black">
<div t-att-class="{'text-muted': i.isCompleted, 'text-decoration-line-through': i.isCompleted}">
<TodoItem todo= "i"/>
<TodoItem todo= "i" onToggleState.bind="toggleState"/>

Choose a reason for hiding this comment

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

Suggested change
<TodoItem todo= "i" onToggleState.bind="toggleState"/>
<TodoItem todo="todo" onToggleState.bind="toggleState"/>

@@ -2,6 +2,7 @@
<templates xml:space="preserve">

<t t-name="awesome_owl.todoitem">
<input type="checkbox" t-on-change="()=> props.onToggleState(props.todo.id)"/>
<t t-out="props.todo.id+'. '+props.todo.description"/>

Choose a reason for hiding this comment

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

Suggested change
<t t-out="props.todo.id+'. '+props.todo.description"/>
<t t-out="props.todo.id + '. ' + props.todo.description"/>

@@ -28,4 +28,11 @@ export class TodoList extends Component {
let todo = this.state.todos.find(todo => todo.id === todoId);
todo.isCompleted = !todo.isCompleted;
}

removeTodo(todoId) {
let index = this.state.todos.findIndex(todo => todo.id === todoId);

Choose a reason for hiding this comment

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

const :)

@msho-odoo msho-odoo force-pushed the 18.0-owl-tutorial-msho branch from c1b4817 to 383d906 Compare May 2, 2025 13:25
@msho-odoo
Copy link
Author

@SergeBayet
Fixed with interactive rebase :)

if (ev.keyCode === 13) {
this.state.idCounter++
this.state.todos.push({
'id': this.state.idCounter, 'description': ev.target.value, 'isCompleted': false

Choose a reason for hiding this comment

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

double quotes :)

setup() {
this.state = useState({
todos: [],
idCounter: 0

Choose a reason for hiding this comment

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

Suggested change
idCounter: 0
idCounter: 0,

name: ('Leads'),
target: 'current',
res_model: 'crm.lead',
views: [[false, 'list'], [false, 'form']],

Choose a reason for hiding this comment

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

double quotes :)


class AwesomeDashboard extends Component {
static template = "awesome_dashboard.AwesomeDashboard";
static components = { Layout }

Choose a reason for hiding this comment

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

Suggested change
static components = { Layout }
static components = { Layout };


class AwesomeDashboard extends Component {
static template = "awesome_dashboard.AwesomeDashboard";
static components = { Layout }
static components = { Layout, DashboardItem }

Choose a reason for hiding this comment

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

Don't forget the semi-colon :)

Suggested change
static components = { Layout, DashboardItem }
static components = { Layout, DashboardItem };

Copy link
Author

Choose a reason for hiding this comment

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

yes, I pushed and then saw the review, the update is on the way :)

@msho-odoo msho-odoo force-pushed the 18.0-owl-tutorial-msho branch from 7952511 to d91de1e Compare May 5, 2025 13:03
Copy link

@MohammedBasioni MohammedBasioni left a comment

Choose a reason for hiding this comment

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

Great work! ;)

<t t-esc="props.title"/>
<button style="border:0.5px solid purple;" class="btn btn-primary" t-on-click="toggleState">Hide/Show</button>
</h5>
<p class="card-text" t-if="state.isOpen">

Choose a reason for hiding this comment

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

better practice to have t-if first

Suggested change
<p class="card-text" t-if="state.isOpen">
<p t-if="state.isOpen" class="card-text">

@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8" ?>
<templates xml:space="preserve">

Choose a reason for hiding this comment

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

not needed

Suggested change
<templates xml:space="preserve">
<templates>

<templates xml:space="preserve">

<t t-name="awesome_owl.counter">
<div style="border:1px solid black; width: 10rem;">Hello World

Choose a reason for hiding this comment

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

Inline styles are not encouraged. You may either use Bootstrap classes directly or create your custom classes and use them.

@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8" ?>

Choose a reason for hiding this comment

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

not needed as this xml is parsed by either Owl or Qweb which already use UTF-8

Suggested change
<?xml version="1.0" encoding="UTF-8" ?>
<?xml version="1.0" ?>

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.

4 participants