-
-
Notifications
You must be signed in to change notification settings - Fork 193
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
[14.0] shopfloor_mobile: improve list-item action handling #582
Conversation
<div class="item-counter" v-if="opts.showCounters"> | ||
<span>{{ index + 1 }} / {{ count }}</span> | ||
</div> | ||
<span v-text="_.result(record, opts.key_title)" /> | ||
<list-item-title-action v-bind="$props" v-if="show_title_action_by_position('right')"/> |
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.
Are this detail action and the one that existed before (v-btn in this component) mutually exclusive? They do the same thing right?
If so, should we hide the v-btn if there's a title action?
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.
I don't get your point. I want to render this action on one side or the other.
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.
Yes, I was just thinking that the detail action you introduce in the title does the same thing as the action in the details part of this component:
<v-btn icon class="detail-action"
v-if="has_detail_action(record, field)"
@click="on_detail_action(record, field, opts)">
<btn-info-icon />
</v-btn>
So maybe we could make them mutually exclusive to avoid making the frontend too crowded. But it's true that this all comes from the conditions in the props so there shouldn't be an issue.
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.
I see. That's another one and - in fact - I could reuse this component there too. I check.
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.
Ok, the scope and the behavior is different, I prefer to not touch it for now.
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.
Sounds good then!
You can now decide if the action goes to the left or the right of the title.
b439341
to
5fadbc2
Compare
/ocabot merge minor |
This PR looks fantastic, let's merge it! |
Congratulations, your PR was merged at 9412b7e. Thanks a lot for contributing to OCA. ❤️ |
You can now decide if the action goes to the left or the right of the title.
ref: cos-3950