[14.0][IMP] web_tree_many2one_clickable: improved visualization #3070

SylweKra · 2025-01-28T12:09:29Z

Before my improvement:
The text was expanding the height of the line, making it harder to maintain consistency and readability across rows.

After my improvement:
The text no longer affects the height of the line, which improves the visibility and alignment of rows.

This adjustment enhances the overall readability and ensures a more structured layout.

francesco-ooops

Functional ok!

@pedrobaeza does code look good?

I have troubles testing on runboat whether the issue is present in later versions

pedrobaeza · 2025-01-30T11:26:47Z

I can't say, as I'm not using anymore this version, and this should be compatible with web_responsive, so a code review can't say.

aleuffre

Code and functional review: mostly seems to work fine, but a few issues.

aleuffre · 2025-02-04T13:54:53Z

web_tree_many2one_clickable/static/src/js/web_tree_many2one_clickable.js

                    html: this.$el.html(),
-                    class: this.$el.attr("class") + " o_field_text",
+                    class: this.$el.attr("class") + "o_field_text",


issue:

Suggested change

class: this.$el.attr("class") + "o_field_text",

class: this.$el.attr("class") + " o_field_text",

aleuffre · 2025-02-04T14:13:00Z

web_tree_many2one_clickable/static/src/css/web_tree_many2one_clickable.scss

+    margin-left: 0.5;
+    visibility: hidden;
+    position: relative;
+    transition: all 0.2s ease;


comment: I find this transition very unpleasant and distracting. Move your mouse across the screen and you have a handful of arrows slowly appearing and disappearing.

I'd personally prefer to have it appear and disappear instantly, as it was before.

I have changed the transition as suggested.

aleuffre · 2025-02-04T15:33:15Z

web_tree_many2one_clickable/static/src/css/web_tree_many2one_clickable.scss


-    &:hover a {


nitpick (non-blocking): I would personally prefer, for readability and maintainability, to make use of scss syntax features, such as nesting selectors etc, as it was before

I have taken care of it as suggested.

suggestion (non-blocking): I probably wasn't very clear. This is what I meant:

td.o_many2one_cell { display: flex; align-items: center; position: relative; a { margin-left: 0.5em; visibility: hidden; position: relative; } &:hover a { visibility: visible; } }

And then you can remove lines 33-37

Also:

.o_many2one_container { display: flex; gap: 0.5em; /* Space between the containers */ .o_text_container { max-width: fit-content; overflow: hidden; white-space: nowrap; text-overflow: ellipsis; } div.o_custom_field_text { position: relative; max-width: fit-content; vertical-align: bottom; } [etc...] }

aleuffre · 2025-02-04T15:38:15Z

web_tree_many2one_clickable/static/src/css/web_tree_many2one_clickable.scss

+    white-space: nowrap;
+    text-overflow: ellipsis;
+}
+td.o_field_text {


issue: o_field_text is a class used in base odoo. The chance of causing issues elsewhere by changing the way it is displayed is quite high.

I think it'd be best to use a different class entirely to add this styling; or at the very least the scope of the selectors should be narrowed down a lot more so that there are (almost) no chances for it to affect unrelated elements in other modules, both custom and from base odoo.

I have added "custom" to the class name to differentiate it from the "field_text" class, preventing any errors, and I have also added the div to ensure a lower scope.

aleuffre

Functionally works well for me now.

Code can be cleaned up (probably even further than what I pointed out in the comments) but is mostly ok.

aleuffre · 2025-02-10T13:53:01Z

web_tree_many2one_clickable/static/src/js/web_tree_many2one_clickable.js

                    html: this.$el.html(),
-                    class: this.$el.attr("class") + " o_field_text",
+                    class: this.$el.attr("class") + " o_custom_field_text",


question: maybe we can have both classes o_field_text and o_custom_field_text?

I can't add both classes; in fact, if I add the o_field_text class, the rows expand again.

aleuffre · 2025-02-10T13:57:23Z

web_tree_many2one_clickable/static/src/css/web_tree_many2one_clickable.scss


-    &:hover a {


suggestion (non-blocking): I probably wasn't very clear. This is what I meant:

td.o_many2one_cell { display: flex; align-items: center; position: relative; a { margin-left: 0.5em; visibility: hidden; position: relative; } &:hover a { visibility: visible; } }

And then you can remove lines 33-37

Also:

.o_many2one_container { display: flex; gap: 0.5em; /* Space between the containers */ .o_text_container { max-width: fit-content; overflow: hidden; white-space: nowrap; text-overflow: ellipsis; } div.o_custom_field_text { position: relative; max-width: fit-content; vertical-align: bottom; } [etc...] }

aleuffre · 2025-02-10T14:00:54Z

web_tree_many2one_clickable/static/src/css/web_tree_many2one_clickable.scss

+}
+/*BUTTON CONTAINER AND BUTTON CSS*/
+.o_button_container {
+    left: 0;


chore: can probably be removed?

I rearranged the classes as you requested.

aleuffre · 2025-02-10T14:01:43Z

web_tree_many2one_clickable/static/src/css/web_tree_many2one_clickable.scss

+.o_text_container {
+    max-width: fit-content;
+    overflow: hidden;
+    white-space: nowrap;


chore (non-blocking): I don't think specifying white-space is necessary since it's already inherited.

I removed the "white-space"; in fact, it was already inherited as you said.

aleuffre · 2025-02-10T14:06:34Z

web_tree_many2one_clickable/static/src/css/web_tree_many2one_clickable.scss

+    left: 0;
+}
+td.o_many2one_cell a {
+    margin-left: 0.5;


chore: missing UoM

Suggested change

margin-left: 0.5;

margin-left: 0.5em;

I added the UoM

aleuffre

Code and functional review, LGTM

francesco-ooops

Functional is looking good

@pedrobaeza how could I check whether it works correctly with web_responsive?

pedrobaeza · 2025-02-11T15:53:11Z

In runboat both should be installed.

francesco-ooops · 2025-02-11T15:57:21Z

Yes, from what I can see they seem to have the same behavior before and after this PR

francesco-ooops · 2025-02-14T09:06:13Z

@OCA/web-maintainers ping :)

francesco-ooops · 2025-02-18T13:48:43Z

@pedrobaeza we verified compatibility with web_responsive both functionally and for code.

Can this be merged? Thanks!

pedrobaeza · 2025-02-18T16:37:27Z

/ocabot merge patch

OCA-git-bot · 2025-02-18T16:37:35Z

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-3070-by-pedrobaeza-bump-patch, awaiting test results.

OCA-git-bot · 2025-02-18T16:41:20Z

Congratulations, your PR was merged at 526845d. Thanks a lot for contributing to OCA. ❤️

pedrobaeza added this to the 14.0 milestone Jan 28, 2025

SylweKra force-pushed the 14.0-IMP-web_tree_many2one_clickable branch from 39a01d2 to a3f4a40 Compare January 29, 2025 08:12

francesco-ooops approved these changes Jan 30, 2025

View reviewed changes

aleuffre suggested changes Feb 4, 2025

View reviewed changes

SylweKra force-pushed the 14.0-IMP-web_tree_many2one_clickable branch 2 times, most recently from c88abfb to 05299f9 Compare February 7, 2025 09:27

aleuffre reviewed Feb 10, 2025

View reviewed changes

[IMP] web_tree_many2one_clickable: improved visualization

7be25dc

SylweKra force-pushed the 14.0-IMP-web_tree_many2one_clickable branch from 05299f9 to 7be25dc Compare February 10, 2025 16:09

aleuffre approved these changes Feb 11, 2025

View reviewed changes

OCA-git-bot added the approved label Feb 11, 2025

francesco-ooops approved these changes Feb 11, 2025

View reviewed changes

OCA-git-bot added the bot is merging ⏳ label Feb 18, 2025

OCA-git-bot merged commit b4e41e3 into OCA:14.0 Feb 18, 2025
8 of 9 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Feb 18, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[14.0][IMP] web_tree_many2one_clickable: improved visualization #3070

[14.0][IMP] web_tree_many2one_clickable: improved visualization #3070

SylweKra commented Jan 28, 2025

francesco-ooops left a comment

pedrobaeza commented Jan 30, 2025

aleuffre left a comment

aleuffre Feb 4, 2025

aleuffre Feb 4, 2025

SylweKra Feb 7, 2025

aleuffre Feb 4, 2025 •

edited

Loading

SylweKra Feb 7, 2025

aleuffre Feb 10, 2025 •

edited

Loading

aleuffre Feb 4, 2025

SylweKra Feb 7, 2025

aleuffre left a comment

aleuffre Feb 10, 2025

SylweKra Feb 10, 2025

aleuffre Feb 10, 2025 •

edited

Loading

aleuffre Feb 10, 2025 •

edited

Loading

SylweKra Feb 10, 2025

SylweKra Feb 10, 2025

aleuffre Feb 10, 2025

SylweKra Feb 10, 2025

aleuffre Feb 10, 2025

SylweKra Feb 10, 2025

aleuffre left a comment

francesco-ooops left a comment

pedrobaeza commented Feb 11, 2025

francesco-ooops commented Feb 11, 2025

francesco-ooops commented Feb 14, 2025

francesco-ooops commented Feb 18, 2025

pedrobaeza commented Feb 18, 2025

OCA-git-bot commented Feb 18, 2025

OCA-git-bot commented Feb 18, 2025

	class: this.$el.attr("class") + "o_field_text",
	class: this.$el.attr("class") + " o_field_text",

[14.0][IMP] web_tree_many2one_clickable: improved visualization #3070

[14.0][IMP] web_tree_many2one_clickable: improved visualization #3070

Conversation

SylweKra commented Jan 28, 2025

francesco-ooops left a comment

Choose a reason for hiding this comment

pedrobaeza commented Jan 30, 2025

aleuffre left a 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

aleuffre Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aleuffre Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aleuffre left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aleuffre Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

aleuffre Feb 10, 2025 • edited Loading

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

aleuffre left a comment

Choose a reason for hiding this comment

francesco-ooops left a comment

Choose a reason for hiding this comment

pedrobaeza commented Feb 11, 2025

francesco-ooops commented Feb 11, 2025

francesco-ooops commented Feb 14, 2025

francesco-ooops commented Feb 18, 2025

pedrobaeza commented Feb 18, 2025

OCA-git-bot commented Feb 18, 2025

OCA-git-bot commented Feb 18, 2025

aleuffre Feb 4, 2025 •

edited

Loading

aleuffre Feb 10, 2025 •

edited

Loading

aleuffre Feb 10, 2025 •

edited

Loading

aleuffre Feb 10, 2025 •

edited

Loading