[9.0] [MIG] website_img_bg_style by cubells · Pull Request #290 · OCA/website

cubells · 2016-11-28T09:48:25Z

Update README.rst
Updated xml files

pedrobaeza · 2016-11-28T20:46:26Z

@@ -0,0 +1,74 @@
+.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg
+    :alt: License: AGPL-3


Put target line

pedrobaeza · 2016-11-28T20:46:45Z

+Known issues / Roadmap
+======================
+
+* Right now you can only select one style for every snippet. To fix that we


Remove this text as we are in version 9 😉

pedrobaeza · 2016-11-28T20:47:14Z

+    "license": "AGPL-3",
+    "application": False,
+    "installable": True,
+    "external_dependencies": {


Don't put empty keys

pedrobaeza · 2016-11-28T20:47:36Z

+    "summary": "Let you change styles of snippets background images",
+    "version": "9.0.1.0.0",
+    "category": "Website",
+    "website": "https://odoo-community.org/",


Put Tecnativa's website

pedrobaeza · 2016-11-28T20:49:03Z

@@ -0,0 +1,357 @@
+@charset "UTF-8";


Don't include this file and map one, as version 9 now generates it on the fly from sass file. Map file isn't generated in fact, but we can't maintain a possible obsolete copy of it.

pedrobaeza · 2016-11-28T20:49:27Z

+              inherit_id="website.assets_frontend"
+              name="Website snippets background image style">
+        <xpath expr="//link[last()]" position="after">
+            <link href="/website_img_bg_style/static/src/css/styles.css"


Put here sass file instead

pedrobaeza

@cubells, I'm afraid the module is not working. I have tried it on runbot and new options don't appear.

cubells · 2016-11-30T06:01:35Z

@pedrobaeza It works for me. Can you check it?

When you save the block, then you can see the new menu "Background style" like this:

pedrobaeza · 2016-11-30T14:48:44Z

OK, there are 2 problems:

Options doesn't appear after you save and edit again. We should be able to access to these options immediately. This also happened on version 8, but it but be very good it it's possible to have them.
Although they appear, they don't work! You can choose to change the alignment, the repeat, whatever, that the background doesn't change. You can check it in runbot v8 http://3273870-8-0-0d7a85.runbot1.odoo-community.org/#scrollTop=0 in contrast with runbot v9 http://3273845-290-56075e.runbot1.odoo-community.org/?db=3273845-290-56075e-all#scrollTop=0

cubells · 2016-11-30T14:52:03Z

ok. I'll review.

cubells · 2016-11-30T18:28:23Z

@pedrobaeza

Options doesn't appear after you save and edit again. We should be able to access to these options immediately. This also happened on version 8, but it but be very good it it's possible to have them.

I see options always once block is saved if block has background.

yajo

Migrating snippets to v9 is one of the hardest parts I found, because there's not much documentation but API has changed a lot.

Let's see if applying these suggestions the addon works fine.

yajo · 2016-12-13T09:21:33Z

+            @if $map2
+                +mix($attribute, $map2, $other: $value)
+
+*


Replace * by .oe_img_bg

yajo · 2016-12-13T09:24:59Z

+          inherit_id="website.snippet_options"
+          name="Container width type chooser">
+    <xpath expr=".">
+        <div data-snippet-option-id='background-style'


Replace data-snippet-option-id='background-style' by data-js="background_style".

yajo · 2016-12-13T09:26:04Z

+                            <li class="dropdown-submenu">
+                                <a href="#">Vertical</a>
+                                <ul class="dropdown-menu">
+                                    <li data-value="bg-top">


Replace every data-value occurrence in this file by data-select_class to take advantage of new features introduced in v9 for snippet options.

Also add an element like this at the end of every option group:

<li data-select_class=""> <a>None</a> </li>

cubells · 2016-12-13T20:11:56Z

All changes requested by @pedrobaeza are done:

Menu is available on edit mode.
Menu now works

yajo · 2016-12-14T08:26:35Z

+          name="Container width type chooser">
+    <xpath expr=".">
+        <div data-js='background-style'
+             data-selector="section, :not(.o_gallery > .container) > .carousel, .parallax">


Ain't it better here to simply put .oe_img_bg? Blocks without background image do not need any of these customizations...

OTOH, another possibility is to put these new options under Background > Style, to remove apparent duplicities as you can see in the image:

OK I saw with you that current behaviour is best possible, so this is OK.

yajo · 2016-12-14T08:27:43Z

+                                        <a>Rounded</a>
+                                    </li>
+                                    <li data-select_class="bg-no-repeat-h">
+                                        <a>None</a>


Here I miss the <li data-select_class=""> option, although you can call it Auto this time, to avoid confusions of having 2 None options.

This was fixed.

yajo · 2016-12-14T08:27:57Z

+                                        <a>Rounded</a>
+                                    </li>
+                                    <li data-select_class="bg-no-repeat-v">
+                                        <a>None</a>


Here I miss the <li data-select_class=""> option, although you can call it Auto this time, to avoid confusions of having 2 None options.

This was fixed.

yajo

All fixed, cool! 👍

yajo · 2016-12-15T09:04:29Z

+          name="Container width type chooser">
+    <xpath expr=".">
+        <div data-js='background-style'
+             data-selector="section, :not(.o_gallery > .container) > .carousel, .parallax">


OK I saw with you that current behaviour is best possible, so this is OK.

yajo · 2016-12-15T09:04:45Z

+                                        <a>Rounded</a>
+                                    </li>
+                                    <li data-select_class="bg-no-repeat-v">
+                                        <a>None</a>


This was fixed.

yajo · 2016-12-15T09:04:51Z

+                                        <a>Rounded</a>
+                                    </li>
+                                    <li data-select_class="bg-no-repeat-h">
+                                        <a>None</a>


This was fixed.

rafaelbn · 2016-12-15T10:08:43Z

Travis fails, could you review please?

yajo · 2016-12-15T10:17:26Z

The failure is in website_seo_redirection, not related

yajo · 2016-12-21T09:47:06Z

Rebasing should fix travis now that #297 is merged.

* Update README.rst * Updated xml files

cubells force-pushed the 9.0-mig-website_img_bg_style branch from d607a99 to 700e21c Compare November 28, 2016 12:12

pedrobaeza force-pushed the 9.0-mig-website_img_bg_style branch from 700e21c to bffc727 Compare November 28, 2016 20:44

pedrobaeza requested changes Nov 28, 2016

View reviewed changes

pedrobaeza mentioned this pull request Nov 28, 2016

Migration to version 9.0 #85

Closed

34 tasks

pedrobaeza requested changes Nov 29, 2016

View reviewed changes

yajo requested changes Dec 13, 2016

View reviewed changes

yajo requested changes Dec 14, 2016

View reviewed changes

yajo approved these changes Dec 15, 2016

View reviewed changes

rafaelbn added this to the 9.0 milestone Dec 15, 2016

rafaelbn added the needs review label Dec 15, 2016

rafaelbn approved these changes Dec 15, 2016

View reviewed changes

Jairo Llopis and others added 4 commits December 21, 2016 12:14

Add module website_img_bg_style.

edab058

Add Sass debug map.

b9e9f69

OCA Transbot updated translations from Transifex

9c7e3ac

[9.0] [MIG] website_img_bg_style

162d99a

* Update README.rst * Updated xml files

cubells force-pushed the 9.0-mig-website_img_bg_style branch from 5dc55e4 to 162d99a Compare December 21, 2016 11:15

pedrobaeza approved these changes Dec 21, 2016

View reviewed changes

pedrobaeza merged commit 3c6c727 into OCA:9.0 Dec 21, 2016

pedrobaeza deleted the 9.0-mig-website_img_bg_style branch December 21, 2016 12:10

		@@ -0,0 +1,74 @@
		.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg
		:alt: License: AGPL-3

Uh oh!

Conversation

cubells commented Nov 28, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pedrobaeza left a comment

Choose a reason for hiding this comment

Uh oh!

cubells commented Nov 30, 2016

Uh oh!

pedrobaeza commented Nov 30, 2016

Uh oh!

cubells commented Nov 30, 2016

Uh oh!

cubells commented Nov 30, 2016

Uh oh!

yajo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubells commented Dec 13, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yajo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rafaelbn commented Dec 15, 2016

Uh oh!

yajo commented Dec 15, 2016

Uh oh!

yajo commented Dec 21, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants