fix: Theming Selected new Font is reverted whenever Color is removed (#36119)

Fixes #15007

/ok-to-test tags="@tag.Theme"


<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10733545727>
> Commit: dec21477152095d034f63a7e08d2719cb0687b12
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10733545727&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Theme`
> Spec:
> <hr>Fri, 06 Sep 2024 06:48:00 UTC
<!-- end of auto-generated comment: Cypress test results  -->


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Enhanced theme validation for the multi-select widget, improving the
robustness of theme-related tests.
- Added new locators for featured themes to enhance testing
capabilities.

- **Bug Fixes**
- Reactivated a previously skipped test case to validate font
consistency across the application.

- **Refactor**
- Optimized color change handling in the ColorPicker component for
better performance and reliability.
- Refactored theme color control logic for improved maintainability and
clarity.

- **Improvements**
- Enhanced type safety in theme properties to prevent misconfigurations.
- Improved testability of the ThemeCard and ThemeSelector components
with new attributes for automated testing.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Pawan Kumar <pawankumar@Pawans-MacBook-Pro-2.local>
This commit is contained in:
Pawan Kumar 2024-09-06 12:46:51 +05:30 committed by GitHub
parent 4a7ff64242
commit f06a82b7bb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 113 additions and 127 deletions

View File

@ -1,6 +1,7 @@
const commonlocators = require("../../../../locators/commonlocators.json");
const themelocator = require("../../../../locators/ThemeLocators.json");
import { multiSelectWidgetLocators } from "../../../../locators/WidgetLocators";
import {
agHelper,
locators,
@ -26,49 +27,19 @@ describe(
agHelper.GetNClick(locators._canvas);
appSettings.OpenAppSettings();
appSettings.GoToThemeSettings();
//Border validation
//cy.contains("Border").click({ force: true });
cy.get(themelocator.border).should("have.length", "3");
cy.borderMouseover(0, "none");
cy.borderMouseover(1, "M");
cy.borderMouseover(2, "L");
cy.get(themelocator.border).eq(1).click({ force: true });
cy.wait("@updateTheme").should(
"have.nested.property",
"response.body.responseMeta.status",
200,
);
cy.wait(1000);
cy.contains("Border").click({ force: true });
//Shadow validation
//cy.contains("Shadow").click({ force: true });
cy.wait(2000);
cy.xpath(theme.locators._boxShadow("L")).click({ force: true });
cy.wait("@updateTheme").should(
"have.nested.property",
"response.body.responseMeta.status",
200,
);
cy.wait(1000);
cy.contains("Shadow").click({ force: true });
//Font
cy.xpath(
"//p[text()='App font']/following-sibling::section//div//input",
).then(($elem) => {
cy.get($elem).click({ force: true });
cy.wait(250);
cy.fixture("fontData").then(function (testdata) {
this.testdata = testdata;
});
agHelper.GetNClick($elem);
cy.get(themelocator.fontsSelected)
//.eq(10)
agHelper
.GetElement(themelocator.fontsSelected)
.should("contain.text", "Nunito Sans");
cy.get(".rc-virtual-list .rc-select-item-option")
.find(".leading-normal")
agHelper
.GetElement(themelocator.fontOption)
.find(themelocator.fontsSelected)
.eq(3)
.then(($childElem) => {
cy.get($childElem).click({ force: true });
@ -80,60 +51,46 @@ describe(
themeFont = `Inter, sans-serif`;
});
});
cy.contains("Font").click({ force: true });
//Color - Bug 23501 - hence skipping
// cy.wait(1000);
// theme.ChangeThemeColor("purple", "Primary");
// cy.get(themelocator.inputColor).should("have.value", "purple");
// cy.wait(1000);
// theme.ChangeThemeColor("brown", "Background");
// cy.get(themelocator.inputColor).should("have.value", "brown");
// cy.wait(1000);
// cy.contains("Color").click({ force: true });
appSettings.ClosePane();
});
//Skipping due to mentioned bug
it.skip("2. Publish the App and validate Font across the app + Bug 15007", function () {
it("2. Publish the App and validate Font across the app", function () {
deployMode.DeployApp();
cy.get(".rc-select-selection-item > .rc-select-selection-item-content")
agHelper
.GetElement(
multiSelectWidgetLocators.multiSelectWidgetSelectedOptionContent,
)
.first()
.should("have.css", "font-family", themeFont);
cy.get(".rc-select-selection-item > .rc-select-selection-item-content")
agHelper
.GetElement(
multiSelectWidgetLocators.multiSelectWidgetSelectedOptionContent,
)
.last()
.should("have.css", "font-family", themeFont);
deployMode.NavigateBacktoEditor();
});
it.skip("3. Validate current theme feature", function () {
cy.get("#canvas-selection-0").click({ force: true });
it("3. Apply theme and validate the color", function () {
appSettings.OpenAppSettings();
appSettings.GoToThemeSettings();
//Change the Theme
cy.get(commonlocators.changeThemeBtn).click({ force: true });
cy.get(themelocator.currentTheme).click({ force: true });
cy.get(".t--theme-card main > main")
.first()
.invoke("css", "background-color")
.then(() => {
cy.get(".t--draggable-multiselectwidgetv2:contains('more')")
.last()
.invoke("css", "background-color")
.then((selectedBackgroudColor) => {
expect("rgba(0, 0, 0, 0)").to.equal(selectedBackgroudColor);
appSettings.ClosePane();
});
});
//Publish the App and validate change of Theme across the app in publish mode
deployMode.DeployApp();
cy.xpath("//div[@id='root']//section/parent::div").should(
"have.css",
"background-color",
"rgb(165, 42, 42)",
agHelper.GetNClick(commonlocators.changeThemeBtn, 0, true);
agHelper.GetNClick(
`${themelocator.featuredThemeSection} [data-testid='t--theme-card-Sunrise']`,
);
deployMode.DeployApp();
agHelper.GetNClick(multiSelectWidgetLocators.multiSelectWidgetTrigger);
agHelper
.GetElement(
multiSelectWidgetLocators.multiSelectWidgetDropdownOptionCheckbox,
)
.first()
.should("have.css", "background-color", "rgb(239, 68, 68)");
});
},
);

View File

@ -9,5 +9,7 @@
"greenColor": "[style='background-color: rgb(21, 128, 61);']",
"fontsSelected": ".leading-normal",
"currentTheme": ".cursor-pointer:contains('Applied theme')",
"purpleColor": "[style='background-color: rgb(107,114,128);']"
}
"purpleColor": "[style='background-color: rgb(107,114,128);']",
"featuredThemeSection": "[data-testid='t--featured-themes']",
"fontOption": ".rc-virtual-list .rc-select-item-option"
}

View File

@ -175,3 +175,9 @@ export const buttongroupwidgetlocators = {
`//*[contains(@class,'bp3-menu-item')]//*[text()='${text}']`,
button: "//*[contains(@class,'t--widget-buttongroupwidget')]//button",
};
export const multiSelectWidgetLocators = {
multiSelectWidgetTrigger: ".t--widget-multiselectwidgetv2 .rc-select-selector",
multiSelectWidgetSelectedOptionContent: ".rc-select-selection-item > .rc-select-selection-item-content",
multiSelectWidgetDropdownOptionCheckbox: ".multi-select-dropdown .rc-select-item-option-selected .bp3-control-indicator"
};

View File

@ -367,12 +367,11 @@ const ColorPickerComponent = React.forwardRef(
defaultFullColorPickerValue,
);
const debouncedOnChange = React.useCallback(
debounce((color: string, isUpdatedViaKeyboard: boolean) => {
const debouncedOnChange = useMemo(() => {
return debounce((color: string, isUpdatedViaKeyboard: boolean) => {
props.changeColor(color, isUpdatedViaKeyboard);
}, DEBOUNCE_TIMER),
[],
);
}, DEBOUNCE_TIMER);
}, [props]);
useEffect(() => {
setIsOpen(isOpenProp);
@ -545,13 +544,19 @@ const ColorPickerComponent = React.forwardRef(
};
}, [handleKeydown]);
const handleChangeColor = (event: React.ChangeEvent<HTMLInputElement>) => {
const value = event.target.value;
if (isValidColor(value)) {
debouncedOnChange(value, true);
}
setColor(value);
};
const handleChangeColor = useCallback(
(event: React.ChangeEvent<HTMLInputElement>) => {
const value = event.target.value;
if (!value) return;
if (isValidColor(value)) {
debouncedOnChange(value, true);
}
setColor(value);
},
[debouncedOnChange],
);
// if props.color changes and state color is different,
// sets the state color to props color

View File

@ -67,7 +67,7 @@ export interface AppThemeProperties {
colors: {
primaryColor: string;
backgroundColor: string;
[key: string]: string;
[key: Exclude<string, number>]: string;
};
borderRadius: {
[key: string]: string;

View File

@ -177,6 +177,7 @@ export function ThemeCard(props: ThemeCard) {
"overflow-hidden": !selectable,
"hover:shadow-xl cursor-pointer": selectable,
})}
data-testid={`t--theme-card-${theme.name}`}
onClick={changeSelectedTheme}
>
<MainContainer backgroundColor={backgroundColor}>

View File

@ -76,7 +76,7 @@ function ThemeEditor() {
dispatch(updateSelectedAppThemeAction({ applicationId, theme }));
},
[updateSelectedAppThemeAction],
[applicationId, dispatch],
);
/**
@ -91,7 +91,7 @@ function ThemeEditor() {
AppThemingMode.APP_THEME_SELECTION,
]),
);
}, [setAppThemingModeStackAction]);
}, [dispatch, themingStack]);
/**
* resets theme

View File

@ -76,7 +76,10 @@ function ThemeSelector() {
))}
</section>
)}
<section className="relative p-4 space-y-3">
<section
className="relative p-4 space-y-3"
data-testid="t--featured-themes"
>
<Title className="text-sm font-medium">Featured themes</Title>
{systemThemes.map((theme) => (
<ThemeCard

View File

@ -1,11 +1,12 @@
import { startCase } from "lodash";
import React, { useState } from "react";
import React, { useCallback, useState } from "react";
import styled from "styled-components";
import type { AppTheme } from "entities/AppTheming";
import { Tooltip } from "@appsmith/ads";
import ColorPickerComponent from "components/propertyControls/ColorPickerComponentV2";
import { capitalizeFirstLetter } from "utils/helpers";
import { objectKeys } from "@appsmith/utils";
interface ThemeColorControlProps {
theme: AppTheme;
@ -33,50 +34,61 @@ function ThemeColorControl(props: ThemeColorControlProps) {
const [selectedColor, setSelectedColor] = useState<string>("primaryColor");
const [isFullColorPicker, setFullColorPicker] = React.useState(false);
const onColorClick = useCallback(
(e: React.MouseEvent<HTMLDivElement>) => {
const colorName = e.currentTarget.getAttribute("data-color-name");
if (!colorName) return;
setAutoFocus(selectedColor === colorName ? !autoFocus : true);
setSelectedColor(colorName);
},
[autoFocus, selectedColor],
);
const onChangeColor = useCallback(
(color: string) => {
updateTheme({
...theme,
properties: {
...theme.properties,
colors: {
...theme.properties.colors,
[selectedColor]: color,
},
},
});
},
[selectedColor, theme, updateTheme],
);
return (
<div className="space-y-2">
<div className="flex space-x-2">
{Object.keys(theme.properties.colors).map(
(colorName: string, index: number) => {
return (
<Tooltip
content={capitalizeFirstLetter(startCase(colorName))}
key={index}
>
<ColorBox
background={userDefinedColors[colorName]}
className={selectedColor === colorName ? "selected" : ""}
data-testid={`theme-${colorName}`}
onClick={() => {
setAutoFocus(
selectedColor === colorName ? !autoFocus : true,
);
setSelectedColor(colorName);
}}
/>
</Tooltip>
);
},
)}
{objectKeys(theme.properties.colors).map((colorName, index) => {
return (
<Tooltip
content={capitalizeFirstLetter(startCase(colorName as string))}
key={index}
>
<ColorBox
background={userDefinedColors[colorName]}
className={selectedColor === colorName ? "selected" : ""}
data-color-name={colorName}
data-testid={`theme-${colorName}`}
onClick={onColorClick}
/>
</Tooltip>
);
})}
</div>
{selectedColor && (
<div className="pt-1 space-y-1">
<h3>{capitalizeFirstLetter(startCase(selectedColor))}</h3>
<ColorPickerComponent
autoFocus={autoFocus}
changeColor={(color: string) => {
updateTheme({
...theme,
properties: {
...theme.properties,
colors: {
...theme.properties.colors,
[selectedColor]: color,
},
},
});
}}
changeColor={onChangeColor}
color={userDefinedColors[selectedColor]}
data-color={selectedColor}
isFullColorPicker={isFullColorPicker}
isOpen={autoFocus}
key={selectedColor}