Skip to content

Conversation

@paru-odoo
Copy link
Contributor

@paru-odoo paru-odoo commented May 28, 2024

Steps to reproduce:

  1. Take any snippet and select any custom gradient color.
  2. Reopen the background color
  • The selected custom gradient color is not retained as expected.

Before version 16, we used wysiwyg, which called the start function to set selected colors easily. In version 17, we switched to OwlJS. Now, color picker always setting the default color as selected color. Therefore, it displays the default color instead of the selected color.

image

After this PR, the selected color will be set in the start function by replacing the default color, and updating RGBA values accordingly.

image

Task-3631963

@robodoo
Copy link
Contributor

robodoo commented May 28, 2024

Pull request status dashboard.

@C3POdoo C3POdoo added the RD research & development, internal work label May 28, 2024
@divy-odoo divy-odoo force-pushed the 17.0-fix-clicking-color-of-custom-gradient-paru branch from 33f9ffb to 8198fb9 Compare June 3, 2024 07:12
@paru-odoo paru-odoo changed the title [FIX] web : resolve custom color gradient issue in owl js conversion [FIX] web : resolve wrongly set default color of color palette Jun 3, 2024
@msh-odoo msh-odoo force-pushed the 17.0-fix-clicking-color-of-custom-gradient-paru branch from 8198fb9 to 07e247a Compare September 10, 2024 11:58
@msh-odoo msh-odoo marked this pull request as ready for review September 10, 2024 11:59
@C3POdoo C3POdoo requested review from a team, BastienFafchamps and aab-odoo and removed request for a team September 10, 2024 12:01
Copy link
Contributor

@aab-odoo aab-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello. The fix looks good. Could you add a test? Thanks

@paru-odoo paru-odoo force-pushed the 17.0-fix-clicking-color-of-custom-gradient-paru branch from 07e247a to 69183e8 Compare September 11, 2024 11:14
@C3POdoo C3POdoo requested a review from a team September 11, 2024 11:16
@paru-odoo paru-odoo force-pushed the 17.0-fix-clicking-color-of-custom-gradient-paru branch 2 times, most recently from a9439f5 to e26d0e9 Compare September 11, 2024 12:03
@paru-odoo paru-odoo changed the title [FIX] web : resolve wrongly set default color of color palette [FIX] web,website: resolve wrongly set default color of color palette Sep 11, 2024
@paru-odoo
Copy link
Contributor Author

Hello @aab-odoo , I have a written test. Could you review once?

@aab-odoo aab-odoo requested a review from FrancoisGe September 12, 2024 13:38
@paru-odoo paru-odoo force-pushed the 17.0-fix-clicking-color-of-custom-gradient-paru branch 2 times, most recently from 48156a8 to 1a09103 Compare September 13, 2024 06:56
@shsa-odoo
Copy link
Contributor

Hello @FrancoisGe This PR is ready and waiting your review on it thanks!

@@ -0,0 +1,69 @@
/** @odoo-module **/

import wTourUtils from "@website/js/tours/tour_utils";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It s probably easier to do a unit test in js. Or I miss something ? It s strange to do a test in website to fix something in web.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @FrancoisGe i believe though the changes are made in colorpicker.js but the functionality these changes have brought is visible in editor or website where the color picker is shown, and in my opinion the test too should be written there only.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FrancoisGe We tried to write the qunit test case but with backend we were not able to produce such case and hence we have written tour for the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't see your ping. If I don't answer after 2 days, you can ping me directly on discord :)

Mmh, in 18.0 we should write a qunit test in community/addons/html_editor/static/tests/color_selector.test.js.
Ok for the tour in 17. (The best solution, it s probably to create a test file specific for ColorPicker)

Hello @FrancoisGe i believe though the changes are made in colorpicker.js but the functionality these changes have brought is visible in editor or website where the color picker is shown, and in my opinion the test too should be written there only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FrancoisGe, We will write QUnit tests in version 18.0. For now, we have created a separate file for the test tour, named colorpicker.js

@shsa-odoo
Copy link
Contributor

Hello @FrancoisGe awaiting your review on this! Thanks!

Steps to reproduce:
1. Take any snippet and select any custom gradient color.
2. Reopen the background color
- The selected custom gradient color is not retained as expected.

Before version 16, we used wysiwyg, which called the start function to
set selected colors easily. In version 17, we switched to OwlJS. Now,
color picker always setting the default color as selected color.
Therefore, it displays the default color instead of the
selected color.

This commit resolves the issue by ensuring the start function sets the
selected color, replacing the default color, and updating RGBA values
accordingly.

Task-3631963
@paru-odoo paru-odoo force-pushed the 17.0-fix-clicking-color-of-custom-gradient-paru branch from 1a09103 to cf518a0 Compare November 7, 2024 03:58
@FrancoisGe
Copy link
Contributor

@robodoo r+

robodoo pushed a commit that referenced this pull request Nov 7, 2024
Steps to reproduce:
1. Take any snippet and select any custom gradient color.
2. Reopen the background color
- The selected custom gradient color is not retained as expected.

Before version 16, we used wysiwyg, which called the start function to
set selected colors easily. In version 17, we switched to OwlJS. Now,
color picker always setting the default color as selected color.
Therefore, it displays the default color instead of the
selected color.

This commit resolves the issue by ensuring the start function sets the
selected color, replacing the default color, and updating RGBA values
accordingly.

Task-3631963

closes #166961

Signed-off-by: Francois Georis (fge) <fge@odoo.com>
@robodoo robodoo closed this Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RD research & development, internal work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants