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

Master forum end qha #29235

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@JKE-be
Contributor

JKE-be commented Dec 4, 2018

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@C3POdoo C3POdoo added the RD label Dec 4, 2018

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Dec 4, 2018

@qsm-odoo qsm-odoo added the Website label Dec 4, 2018

@qsm-odoo qsm-odoo self-requested a review Dec 4, 2018

@qsm-odoo

Result seems nice but the code has still lots of room for improvements, please try to check and we will discuss tomorrow about it.

Show resolved Hide resolved addons/website_forum/data/forum_default_faq.html Outdated
Show resolved Hide resolved addons/website_forum/data/forum_default_faq.html Outdated
@@ -44,7 +43,7 @@ def _get_default_faq(self):
# description and use
name = fields.Char('Forum Name', required=True, translate=True)
active = fields.Boolean(default=True)
faq = fields.Html('Guidelines', default=_get_default_faq, translate=True)
faq = fields.Html('Guidelines', default=_get_default_faq, translate=True, sanitize=False)

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 4, 2018

Contributor

@JKE-be I have a dream: odoo-dev@33c2758

Show resolved Hide resolved addons/website_forum/static/src/js/website_forum.js Outdated
Show resolved Hide resolved addons/website_forum/static/src/js/website_forum.js Outdated
Show resolved Hide resolved addons/website_forum/static/src/xml/website_forum_share_templates.xml Outdated
Show resolved Hide resolved addons/website_forum/views/website_forum.xml Outdated
Show resolved Hide resolved addons/website_forum/views/website_forum.xml Outdated
Show resolved Hide resolved addons/website_forum/views/website_forum.xml Outdated
Show resolved Hide resolved addons/website_forum/views/website_forum.xml Outdated

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Dec 5, 2018

@qha-odoo qha-odoo force-pushed the odoo-dev:master-forum-end-qha branch from 1d23f07 to 8eddc05 Dec 5, 2018

@robodoo robodoo removed the CI 🤖 label Dec 5, 2018

@qha-odoo qha-odoo force-pushed the odoo-dev:master-forum-end-qha branch from a7563d4 to 98e5541 Dec 5, 2018

Show resolved Hide resolved addons/website_forum/static/src/js/website_forum.js Outdated
Show resolved Hide resolved addons/website_forum/static/src/js/website_forum.js Outdated
Show resolved Hide resolved addons/website_forum/static/src/js/website_forum.js Outdated
Show resolved Hide resolved addons/website_forum/static/src/scss/website_forum.scss Outdated
Show resolved Hide resolved addons/website_forum/static/src/scss/website_forum.scss Outdated
<span t-if="not question.active and question.state=='offensive' and question.closed_reason_id"><b> [<t t-esc="question.closed_reason_id.name[0].upper() + question.closed_reason_id.name[1:]"/>]</b></span>
<span t-if="question.state == 'close'"><b> [Closed]</b></span>
</div>
<div><t t-raw="post_content"/></div>

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 5, 2018

Contributor

raw ?

Show resolved Hide resolved addons/website_forum/views/website_forum.xml Outdated
Show resolved Hide resolved addons/website_forum/views/website_forum.xml Outdated
Show resolved Hide resolved addons/website_forum/views/website_forum.xml Outdated
<div t-attf-class="forum_answer card" t-attf-id="answer_#{answer.id}" >
<div class="card-body">
<div class="row no-gutters">
<div t-if="answer.is_correct" class="validate_answer">

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 5, 2018

Contributor

😢

spaces ... columns in row ...

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭😭

@qha-odoo qha-odoo force-pushed the odoo-dev:master-forum-end-qha branch from beda89a to 5733faa Dec 6, 2018

@qsm-odoo

Big batch of new comments. You also ignored some comments I made yesterday about spacing and other things. Also, when I make a comment like "col must be in a row", it means at all places not only on the line I commented. Could you check please? Thank you.

<div class="row">
<div class="col-lg-12">
<h1 class="text-center">Welcome!</h1>
<p class="text-400 text-center col-lg-8 mx-auto">

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

col on a p in a col...

<div class="col-lg-12">
<a href="#" class="js_close_intro">Hide Intro</a> <a class="btn btn-primary forum_register_url" href="/web/login">Register</a> </div>
default="""<section class="bg-info shadow">
<div class="container p-5">

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

You cannot change the horizontal padding of a container !

Show resolved Hide resolved addons/website_forum/static/src/js/website_forum.js
Show resolved Hide resolved addons/website_forum/static/src/js/website_forum.js
Show resolved Hide resolved addons/website_forum/static/src/js/website_forum.js Outdated
<div class="col-md-2">
<img class="img img-fluid rounded-circle" t-attf-src="/forum/user/#{user.id}/avatar" alt="Avatar"/>
<div class="mb-4 border-bottom pb-2">
<h1 class="o_page_header d-inline border-0">Profile</h1>

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

an inline o_page_header canceled by a border-0 in a div with border followed by a floating link...
Is not this working putting the floating link before the h1, removing the inline stuff and removing the div ?

<t t-if="user.country_id"> <span t-field="user.country_id.image" t-options='{"widget": "image", "class": "country_flag"}' class="mr-2"/></t>
<span t-field="user.city"/>
<span t-if="user.city and user.country_id">, </span>
<span t-field="user.country_id"/>

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

Stuff in row which are not col... again.

<span t-if="user.city and user.country_id">, </span>
<span t-field="user.country_id"/>
</div>
<div class="border-top pt-2 mt-2 row">

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

A row with border has probably not the effect you want. What if we have a theme with big gutters ?

<span t-esc="post[0].name"/>
</a>
</span>
<li t-foreach="activities" t-as="activity" class="card mb-2">

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

Why using cards on <li/> elements? Why using a list to display cards?

</span>
<li t-foreach="activities" t-as="activity" class="card mb-2">
<div class="card-body">
<div>

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

What is the purpose of that div ?

@qsm-odoo

qsm-odoo requested changes Dec 6, 2018 edited

Quick look, still stuff to change but I guess we have a month to do so :/

<div class="row">
<div class="col-lg-12">
<h1 class="text-center">Welcome!</h1>
<p class="text-400 text-center">

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

We should not use text-400 inside a bg-info since if the user changes the info color to light color.... this will break... but well, I guess this is fixable by the user :/

read_events: {
'click .o_wforum_select_all_spam': '_onSelectallSpamClick',
'click .o_wforum_mark_spam': '_onMarkSpamClick',
'input #spamSearch': '_onSpamSearchInput',

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

@JKE-be Should we add the async stuffs ? :)

var values = _.map($inputs, function (o) {
return parseInt(o.value);
});
var self = this;

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

No need of self here :)

<t t-if="question.post_type != 'question'">
<t t-if="question.child_count&lt;=1">Comment</t>
<t t-else="">Comments</t>
<div t-if="question.has_validated_answer" class="validate_answer">

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

Clearly this should not be there as its neighbors seems to be a col and this is not a col 😭

<a href="#" t-if="queue_type == 'offensive'" disabled="True" aria-label="Offensive" title="Offensive"><i class="fa fa-times fa-2x" style="color:rgb(179, 179, 179);"/></a>
<div class="modal fade" t-att-data-spam-ids="str(posts_ids.ids)" id="markAllAsSpam" tabindex="-1" role="dialog" aria-labelledby="markAllAsSpam" aria-hidden="true">
<div class="modal-dialog" role="document">
<div class="modal-dialog modal-content">

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

wrong modal structure

</div>
</div>
</div>
<div class="modal-footer justify-content-start">

This comment has been minimized.

@qsm-odoo

qsm-odoo Dec 6, 2018

Contributor

We should not change footer button alignement (at least not case by case)

@JKE-be JKE-be force-pushed the odoo-dev:master-forum-end-qha branch 2 times, most recently from 25555ab to d2dc205 Dec 6, 2018

@robodoo robodoo added the CI 🤖 label Dec 6, 2018

[REF] website_forum: mass moderation, modern UI bs4 and clean type
Now forum only allow to post message of type 'question'.
New 'modern' UI with Bootstrap 4
New moderation modal for bulk spam

Co-authored-by: qha <qha@odoo.com>
Co-authored-by: qsm <qsm@odoo.com>
Co-authored-by: jke <jke@odoo.com>

Thanks to @qha-odoo for UI
Thanks to @qsm-odoo for reviewSsss

@JKE-be JKE-be force-pushed the odoo-dev:master-forum-end-qha branch from d2dc205 to 0d050c7 Dec 6, 2018

@robodoo robodoo removed the CI 🤖 label Dec 6, 2018

@JKE-be

This comment has been minimized.

Contributor

JKE-be commented Dec 6, 2018

@robodoo r+ rebase-ff

robodoo pushed a commit that referenced this pull request Dec 6, 2018

[REF] website_forum: mass moderation, modern UI bs4 and clean type
Now forum only allow to post message of type 'question'.
New 'modern' UI with Bootstrap 4
New moderation modal for bulk spam

Co-authored-by: qha <qha@odoo.com>
Co-authored-by: qsm <qsm@odoo.com>
Co-authored-by: jke <jke@odoo.com>

Thanks to @qha-odoo for UI
Thanks to @qsm-odoo for reviewSsss

closes #29235

@robodoo robodoo added merging 👷 and removed merging 👷 labels Dec 6, 2018

@robodoo robodoo added the merged 🎉 label Dec 6, 2018

@robodoo

This comment has been minimized.

Contributor

robodoo commented Dec 6, 2018

Merged, thanks!

@robodoo robodoo closed this Dec 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment