Begin of the invisible elements #3574

loco-odoo · 2024-11-25T13:48:59Z

No description provided.

robodoo · 2024-11-25T13:49:03Z

This PR targets the un-managed branch odoo-dev/odoo:master-mysterious-egg, it needs to be retargeted before it can be merged.

Steps to reproduce: - Enter in edit mode. - Select the "Customize" tab. -> Error.

FrancoisGe · 2024-11-27T15:30:13Z

addons/html_builder/static/src/builder/plugins/drop_zone_plugin.js

+            for (const handler of this.getResource("on_snippet_dropped")) {
+                handler();
+            }


use this.dispatchTo("on_snippet_dropped")

FrancoisGe · 2024-11-27T15:32:12Z

addons/html_builder/static/src/builder/snippets_menu_tabs/invisible_elements_tab.js

+    }
+}
+
+function isSnippetVisible(snippetEl) {


isVisible(el) ?

FrancoisGe · 2024-11-27T15:32:46Z

addons/html_builder/static/src/builder/snippets_menu_tabs/invisible_elements_tab.js

+    return snippetEl.dataset.invisible !== "1";
+}
+
+function setSnippetVisibility(invisibleEntry, show) {


setVisibility(el, visible)

Goaman · 2024-11-27T14:41:23Z

addons/html_builder/static/src/builder/plugins/drop_zone_plugin.js

+            for (const handler of this.getResource("on_snippet_dropped")) {
+                handler();
+            }


I would use the editor config onChange instead of a new signal on_snippet_dropped, otherwise undo/redo will not be taken into consideration.

Goaman · 2024-11-27T14:48:47Z

addons/html_builder/static/src/builder/snippet_model.js

@@ -78,6 +78,7 @@ export class SnippetModel extends Reactive {
                    isCustom: false,
                    imagePreviewSrc: snippetEl.dataset.oImagePreview,
                };
+                snippetEl.children[0].dataset["name"] = snippet.title;


that is strange to mutate the dom in a method called "compute", if necessary I would create another method.

Why do need to duplicate the information in the same node? Either
2.1 We change the xml to change the key data-snippet to data-name or data-snippet-name
2.2 The code that reads for the name read data-name || data-snippet

If I had to chose, I'd take 2.1, because it makes more sense to have the data only once with a consistent key. Except if there is a difference in meaning with the 2 keys (data-snippet and data-name).

Goaman · 2024-11-27T14:49:28Z

addons/html_builder/static/src/builder/snippets_menu.js

@@ -86,6 +92,7 @@ export class SnippetsMenu extends Component {
                        this.state.currentOptionsContainers = currentOptionsContainers;
                        this.setTab("customize");
                    },
+                    on_snippet_dropped: this.updateInvisibleEls.bind(this),


You use the editor config onChange to take the history in consideration.

Goaman · 2024-11-27T14:54:06Z

addons/html_builder/static/src/builder/snippets_menu.scss

@@ -22,3 +22,48 @@
    background-color: #3e3e46;
 }

+.o_we_invisible_el_panel {


Part of the refactoring is to remove css that should be converted to bootstrap classes so you should add the bootsrap classes directly in the component xml template whenever possible.

For precise position & size, we haven't concluded anything yet, I'm not sure of the current team vision on that. cc @FrancoisGe

Goaman · 2024-11-27T14:56:17Z

addons/html_builder/static/src/builder/snippets_menu_tabs/invisible_elements_tab.js

+import { Component, onWillStart, onWillUpdateProps, useState } from "@odoo/owl";
+import { WithSubEnv } from "../builder_helpers";
+import { getSnippetName } from "../../utils";
+export class InvisibleElementsTab extends Component {


I would not mention the concept of "Tab" in the name as it is not a Tab.
InvisibleElements ?

Goaman · 2024-11-27T15:23:36Z

addons/html_builder/static/src/builder/snippets_menu.js

@@ -235,6 +248,12 @@ export class SnippetsMenu extends Component {
        }
        return true;
    }
+
+    updateInvisibleEls() {
+        this.state.invisibleEls = [


invisibleElements to avoid abbreviation

Goaman · 2024-11-27T15:25:24Z

addons/html_builder/static/src/builder/snippets_menu_tabs/invisible_elements_tab.js

+    };
+
+    setup() {
+        this.state = useState({ invisibleEls: null });


invisibleEntries ?

Goaman · 2024-11-27T15:39:41Z

addons/html_builder/static/src/builder/snippets_menu_tabs/invisible_elements_tab.js

+        this.state.invisibleEls = createInvisibleElements(rootInvisibleSnippetEls, false);
+    }
+
+    onInvisibleEntryClick(invisibleEntry) {


toggleElementVisibility(element)?

Goaman · 2024-11-27T15:42:24Z

addons/html_builder/static/src/builder/snippets_menu_tabs/invisible_elements_tab.xml

+            Invisible Elements
+        </div>
+        <t t-foreach="state.invisibleEls" t-as="invisibleEntry" t-key="invisibleEntry_index">
+            <t t-call="html_builder.invisibleSnippetEntry" t-call-context="{'entry': invisibleEntry, 'invisibleElTab': this}"/>


instead of 'invisibleElTab': this, what about 'toggleElementVisibility': toggleElementVisibility

Goaman · 2024-11-27T15:54:21Z

addons/html_builder/static/tests/block_tab/snippet_groups.test.js

-}
-
-function getBasicSection(content) {
+function getBasicSection(content, name, processed = false) {


Another reason why not to have some processing changing the template, so we don't have "convoluted" checks.

If it is really necessary, you could always return the "processed" to avoid to having to write getBasicSection(..., false) and getBasicSection(..., true).

Just a side note and detail, I would always avoid boolean arguments and have it wrapped in an object instead. Some arguments can be found here: https://alexkondov.com/should-you-pass-boolean-to-functions/

loco-odoo force-pushed the master-mysterious-egg-loco-1 branch 9 times, most recently from 9c4544a to fa9f7b0 Compare November 27, 2024 12:43

loco-odoo added 2 commits November 27, 2024 15:52

Small fix

bec31f8

Steps to reproduce: - Enter in edit mode. - Select the "Customize" tab. -> Error.

Begin of the invisible elements

80f3b63

loco-odoo force-pushed the master-mysterious-egg-loco-1 branch from fa9f7b0 to 80f3b63 Compare November 27, 2024 14:54

FrancoisGe reviewed Nov 27, 2024

View reviewed changes

Goaman reviewed Nov 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Begin of the invisible elements #3574

Begin of the invisible elements #3574

loco-odoo commented Nov 25, 2024

robodoo commented Nov 25, 2024

FrancoisGe Nov 27, 2024

FrancoisGe Nov 27, 2024

FrancoisGe Nov 27, 2024

Goaman Nov 27, 2024

Goaman Nov 27, 2024

Goaman Nov 27, 2024

Goaman Nov 27, 2024

Goaman Nov 27, 2024

Goaman Nov 27, 2024

Goaman Nov 27, 2024

Goaman Nov 27, 2024

Goaman Nov 27, 2024

Goaman Nov 27, 2024

Begin of the invisible elements #3574

Are you sure you want to change the base?

Begin of the invisible elements #3574

Conversation

loco-odoo commented Nov 25, 2024

robodoo commented Nov 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment