diff --git a/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts b/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts index e52de8566019..b5daff34182d 100644 --- a/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts +++ b/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts @@ -1621,11 +1621,25 @@ export default class StyleEditor { // Make a new style. Initialize to all current values. Caller should ensure it is a valid new style. public createStyle() { const typedStyle = $("#style-select-input").val(); - StyleEditor.SetStyleNameForElement( - this.boxBeingEdited, - typedStyle + "-style", - ); + // A box's font normally comes from the collection's language settings, not from its style, + // so the new style says nothing about the font unless the old style did: only a font the + // user set explicitly carries over, per language, for every language in the group (the + // whole group moves to the new style). Read them before the style changes. + const explicitFonts = this.getExplicitFontsForGroup(); + const newStyleName = typedStyle + "-style"; + StyleEditor.SetStyleNameForElement(this.boxBeingEdited, newStyleName); this.updateStyle(); + if (explicitFonts.size > 0) { + explicitFonts.forEach((font, languageSelector) => { + const rule = this.GetRuleForStyle( + newStyleName, + languageSelector, + true, + ); + rule?.style.setProperty("font-family", font, "important"); + }); + this.cleanupAfterStyleChange(); + } // Recommended way to insert an item into a select2 control and select it (one of the trues makes it selected) // See http://codepen.io/alexweissman/pen/zremOV @@ -1640,6 +1654,47 @@ export default class StyleEditor { $("#style-select-input").val(""); } + /** + * The fonts the box's style sets explicitly (the rules changeFont writes), for the box being + * edited and every other language's box in its translation group: a map from the language + * part of the selector ('[lang="fr"]', or "" for a box that uses language-independent rules) + * to the font. A language whose font comes from the collection's settings is not in the map. + * Reads only; never creates a rule. + */ + private getExplicitFontsForGroup(): Map { + const fonts = new Map(); + const target = this.boxBeingEdited; + const styleName = StyleEditor.GetStyleNameForElement(target); + if (!styleName) { + return fonts; + } + const group = target.closest(".bloom-translationGroup"); + const boxes = group + ? Array.from(group.getElementsByClassName("bloom-editable")) + : [target]; + for (const box of boxes) { + let languageSelector = ""; + if (!this.targetUsesLanguageIndependentRules(box as HTMLElement)) { + const lang = StyleEditor.GetLangValueOrNull(box as HTMLElement); + languageSelector = lang + ? '[lang="' + lang + '"]' + : ":not([lang])"; + } + const rule = this.GetRuleForStyle( + styleName, + languageSelector, + false, + ); + const font = rule?.style.getPropertyValue("font-family"); + if (font) { + fonts.set(languageSelector, font); + } + } + return fonts; + } + + // Copy every control's current value into the box's (new) style. The font is deliberately + // not among them: see createStyle. public updateStyle() { this.changeSize(); this.changeLineheight(); @@ -1874,11 +1929,22 @@ export default class StyleEditor { if (this.ignoreControlChanges) { return; } - const rule = this.getStyleRule(false); + // Like the other Characters-tab controls (bold, size, spacing...): the color always goes + // into the language-specific rule, and when the box being edited is in the collection's + // first language it goes into the language-independent rule as well, so that the other + // languages of the style pick it up too (BL-16803). Font family is the one deliberate + // exception to that pattern, because a font suits a script, not a style. + let rule = this.getStyleRule(false); if (rule != null) { rule.style.setProperty("color", color); - this.cleanupAfterStyleChange(); } + if (this.shouldSetDefaultRule()) { + rule = this.getStyleRule(true); + if (rule != null) { + rule.style.setProperty("color", color); + } + } + this.cleanupAfterStyleChange(); this.setColorButtonColor("colorSelectButton", color); } diff --git a/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditorSpec.ts b/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditorSpec.ts index 9b438a8341df..d0f85b9d200e 100644 --- a/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditorSpec.ts +++ b/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditorSpec.ts @@ -496,6 +496,211 @@ describe("StyleEditor", () => { } }); + // The Format dialog's Color control follows the same rule as bold, size and spacing: a change + // made on a box in the collection's first language is for the style as a whole, so it goes + // into the language-independent rule too (BL-16803). Font family is the deliberate exception. + it("changeColor on a first-language box writes the color to the language-specific and the language-independent rules", () => { + (globalThis as any).GetSettings = () => ({ + languageForNewTextBoxes: "xyz", + }); + try { + $("body").append( + "
" + + "
", + ); + const editor = new StyleEditor( + "file://" + "C:/dev/Bloom/src/BloomBrowserUI/bookEdit", + ); + editor.boxBeingEdited = $("#testTarget").get(0); + vi.spyOn(editor, "cleanupAfterStyleChange").mockImplementation( + () => {}, + ); + // sanity check: nothing has written a color yet + expect(GetRuleMatchingSelector("color:")).toBeNull(); + + editor.changeColor("rgb(255, 22, 22)"); + + expect( + GetRuleMatchingSelector('.foo-style[lang="xyz"]')?.cssText, + ).toContain("color: rgb(255, 22, 22)"); + expect(GetRuleMatchingSelector(".foo-style {")?.cssText).toContain( + "color: rgb(255, 22, 22)", + ); + expect( + $("#colorSelectButton").attr("style"), + "the dialog's color button shows the new color", + ).toContain("rgb(255, 22, 22)"); + } finally { + delete (globalThis as any).GetSettings; + } + }); + + it("changeColor on a box in another language writes the color only to that language's rule", () => { + (globalThis as any).GetSettings = () => ({ + languageForNewTextBoxes: "xyz", + }); + try { + $("body").append( + "
" + + "
", + ); + const editor = new StyleEditor( + "file://" + "C:/dev/Bloom/src/BloomBrowserUI/bookEdit", + ); + editor.boxBeingEdited = $("#testTarget").get(0); + vi.spyOn(editor, "cleanupAfterStyleChange").mockImplementation( + () => {}, + ); + + editor.changeColor("rgb(255, 22, 22)"); + + expect( + GetRuleMatchingSelector('.foo-style[lang="abc"]')?.cssText, + ).toContain("color: rgb(255, 22, 22)"); + expect( + GetRuleMatchingSelector(".foo-style {"), + "no language-independent rule should be written for a non-L1 box", + ).toBeNull(); + } finally { + delete (globalThis as any).GetSettings; + } + }); + + // The controls createStyle copies into the new style. The values do not matter here; they + // only have to exist, because updateStyle reads every one of them. + const formatDialogControlsHtml = + "" + + "" + + "" + + "" + + "
" + + "
" + + "
" + + "" + + "
" + + ""; + + // A box's font normally comes from the collection's language settings, not from its style, + // so a new style should say nothing about the font unless the old style set one explicitly. + it("createStyle copies a font the old style set explicitly for the box's language", () => { + (globalThis as any).GetSettings = () => ({ + languageForNewTextBoxes: "xyz", + }); + try { + $("body").append( + "
" + + formatDialogControlsHtml, + ); + const editor = new StyleEditor( + "file://" + "C:/dev/Bloom/src/BloomBrowserUI/bookEdit", + ); + editor.boxBeingEdited = $("#testTarget").get(0); + vi.spyOn(editor, "cleanupAfterStyleChange").mockImplementation( + () => {}, + ); + editor.changeFont("Arial"); + // sanity check: the old style now names the font for this language + expect( + GetRuleMatchingSelector('.foo-style[lang="xyz"]')?.cssText, + ).toContain("font-family: Arial"); + + // runFormatDialog fills the style list when the dialog opens; createStyle adds to it. + (editor as any).styles = []; + editor.createStyle(); + + expect($("#testTarget").attr("class")).toContain("Bar-style"); + expect( + GetRuleMatchingSelector('.Bar-style[lang="xyz"]')?.cssText, + ).toContain("font-family: Arial"); + // The font stays per language: the language-independent rule says nothing about it. + expect( + GetRuleMatchingSelector(".Bar-style {")?.cssText, + ).not.toContain("font-family"); + } finally { + delete (globalThis as any).GetSettings; + } + }); + + it("createStyle leaves the font to the language's default when the old style did", () => { + (globalThis as any).GetSettings = () => ({ + languageForNewTextBoxes: "xyz", + }); + try { + $("body").append( + "
" + + formatDialogControlsHtml, + ); + const editor = new StyleEditor( + "file://" + "C:/dev/Bloom/src/BloomBrowserUI/bookEdit", + ); + editor.boxBeingEdited = $("#testTarget").get(0); + vi.spyOn(editor, "cleanupAfterStyleChange").mockImplementation( + () => {}, + ); + // sanity check: nothing names a font yet + expect(GetRuleMatchingSelector("font-family")).toBeNull(); + + // runFormatDialog fills the style list when the dialog opens; createStyle adds to it. + (editor as any).styles = []; + editor.createStyle(); + + expect($("#testTarget").attr("class")).toContain("Bar-style"); + // The new style got the other settings... + expect( + GetRuleMatchingSelector('.Bar-style[lang="xyz"]')?.cssText, + ).toContain("font-size: 12pt"); + // ...but no font, in any of its rules. + expect(GetRuleMatchingSelector("font-family")).toBeNull(); + } finally { + delete (globalThis as any).GetSettings; + } + }); + + it("createStyle keeps the explicit font of every language in the box's translation group", () => { + (globalThis as any).GetSettings = () => ({ + languageForNewTextBoxes: "xyz", + }); + try { + $("body").append( + "
" + + "
" + + "
" + + "
" + + formatDialogControlsHtml, + ); + const editor = new StyleEditor( + "file://" + "C:/dev/Bloom/src/BloomBrowserUI/bookEdit", + ); + vi.spyOn(editor, "cleanupAfterStyleChange").mockImplementation( + () => {}, + ); + // The user chose a different font for each language. + editor.boxBeingEdited = $("#sibling").get(0); + editor.changeFont("Verdana"); + editor.boxBeingEdited = $("#testTarget").get(0); + editor.changeFont("Arial"); + // sanity check + expect( + GetRuleMatchingSelector('.foo-style[lang="abc"]')?.cssText, + ).toContain("font-family: Verdana"); + + // runFormatDialog fills the style list when the dialog opens; createStyle adds to it. + (editor as any).styles = []; + editor.createStyle(); + + // Both boxes moved to the new style, and each keeps its own font. + expect($("#sibling").attr("class")).toContain("Bar-style"); + expect( + GetRuleMatchingSelector('.Bar-style[lang="xyz"]')?.cssText, + ).toContain("font-family: Arial"); + expect( + GetRuleMatchingSelector('.Bar-style[lang="abc"]')?.cssText, + ).toContain("font-family: Verdana"); + } finally { + delete (globalThis as any).GetSettings; + } + }); + it("UpdateControlsToReflectAppliedStyle passes the real highlight colors to changeHiliteProps", () => { $("body").append( "
", diff --git a/src/BloomE2E/helpers/bookMaking.ts b/src/BloomE2E/helpers/bookMaking.ts index 7a2830f9ae2b..cd425c03a2f6 100644 --- a/src/BloomE2E/helpers/bookMaking.ts +++ b/src/BloomE2E/helpers/bookMaking.ts @@ -521,7 +521,8 @@ export async function goToPage(page: Page, pageId: string): Promise { /** * Click in one language's box of one translation group on the page being shown, so that it has the * focus, the way a person starts editing it. `groupSelector` picks the group, e.g. ".bookTitle" for - * the cover title. Waits until the box has the focus, and returns it. + * the cover title; when the page has several groups that match, `groupIndex` says which one, in + * document order. Waits until the box has the focus, and returns it. * * Focusing a box is also what makes Bloom show the box's format gear (see helpers/formatDialog.ts). */ @@ -529,9 +530,12 @@ export async function clickInGroup( page: Page, groupSelector: string, languageTag: string, + groupIndex = 0, ): Promise { const box = editablePageFrame(page) - .locator(`${groupSelector} .bloom-editable[lang="${languageTag}"]`) + .locator(groupSelector) + .nth(groupIndex) + .locator(`.bloom-editable[lang="${languageTag}"]`) .first(); await box.waitFor({ state: "visible", timeout: 30000 }); await box.click(); @@ -544,9 +548,11 @@ export async function clickInGroup( /** * Type text into one language's box of one translation group on the page being shown, the way a - * person does. `groupSelector` picks the group, e.g. ".bookTitle" for the cover title. + * person does. `groupSelector` picks the group, e.g. ".bookTitle" for the cover title; when the + * page has several groups that match, `groupIndex` says which one, in document order. * - * Pass an empty string to clear the box; that is how a test makes a translation incomplete. + * Pass an empty string to clear the box; that is how a test makes a translation incomplete. A + * newline in `text` presses Enter, which starts a new paragraph, as it does for a person. * Nothing reaches the file until the book leaves this page — see goToPage. */ export async function typeInGroup( @@ -554,16 +560,23 @@ export async function typeInGroup( groupSelector: string, languageTag: string, text: string, + groupIndex = 0, ): Promise { // Click in, select what is there, and type over it. A box here is a CKEditor surface, and // filling it directly leaves part of the old text behind. - const box = await clickInGroup(page, groupSelector, languageTag); + const box = await clickInGroup( + page, + groupSelector, + languageTag, + groupIndex, + ); await box.press("Control+a"); await box.press("Delete"); if (text) await box.pressSequentially(text); // Bloom's editor reacts to typing; confirm the box holds what we meant before moving on, so a - // later failure cannot be blamed on text that never arrived. - await expect(box).toHaveText(text, { timeout: 15000 }); + // later failure cannot be blamed on text that never arrived. innerText, rather than textContent, + // so that a paragraph break reads back as the newline that made it. + await expect(box).toHaveText(text, { timeout: 15000, useInnerText: true }); } /** One front or back matter page, as the Edit tab showed it. */ diff --git a/src/BloomE2E/helpers/cssValues.ts b/src/BloomE2E/helpers/cssValues.ts new file mode 100644 index 000000000000..dce56accfa6a --- /dev/null +++ b/src/BloomE2E/helpers/cssValues.ts @@ -0,0 +1,43 @@ +// Turn the values a browser reports for computed styles into the units a person sees in Bloom's UI. +// +// A primitive: surface modules use these when they read the page, so that a test compares what it +// asked for ("#ff1616", 17 points) with what the page shows, in the same terms, and never parses +// "rgb(255, 22, 22)" or "22.6667px" itself. + +/** + * A CSS color as the browser reports it ("rgb(255, 22, 22)", "rgba(0, 0, 0, 1)", "#FF1616", + * "transparent") as lower-case "#rrggbb", or "transparent" when the color is fully transparent. + * Throws for anything else, naming what it was given. + */ +export function cssColorToHex(cssColor: string): string { + const value = cssColor.trim(); + if (value === "transparent") return "transparent"; + const rgb = + /^rgba?\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*(?:,\s*([\d.]+)\s*)?\)$/.exec( + value, + ); + if (rgb) { + if (rgb[4] !== undefined && Number(rgb[4]) === 0) return "transparent"; + return ( + "#" + + [rgb[1], rgb[2], rgb[3]] + .map((n) => Number(n).toString(16).padStart(2, "0")) + .join("") + ); + } + const hex = /^#([0-9a-f]{6})([0-9a-f]{2})?$/i.exec(value); + if (hex) { + if (hex[2] !== undefined && parseInt(hex[2], 16) === 0) + return "transparent"; + return "#" + hex[1].toLowerCase(); + } + throw new Error(`"${cssColor}" is not a color this helper understands.`); +} + +/** Pixels (as computed styles report them, e.g. "22.6667px") to points, rounded to 2 decimals. */ +export function cssPxToPt(cssLength: string): number { + const px = parseFloat(cssLength); + if (Number.isNaN(px)) + throw new Error(`"${cssLength}" is not a length in pixels.`); + return Math.round(px * 0.75 * 100) / 100; +} diff --git a/src/BloomE2E/helpers/formatDialog.ts b/src/BloomE2E/helpers/formatDialog.ts index bd042094c3d8..74ac5308d521 100644 --- a/src/BloomE2E/helpers/formatDialog.ts +++ b/src/BloomE2E/helpers/formatDialog.ts @@ -11,9 +11,24 @@ // The gear is clicked at a point, not with Playwright's own click, because Playwright scrolls its // target into view first, and the case the manual test cares about most is a gear that is only // partly in view. +// +// The second half of this file drives the dialog's four tabs (Style Name, Characters, Paragraph, +// Highlighting) and reads what they did to the text boxes on the page. What a control changes: +// +// - Every control writes CSS rules for the box's STYLE ("normal", or one the user created) into +// the book's userModifiedStyles sheet, so it changes every box of that style, on every page. +// - A Characters-tab change made on a box in the collection's first language goes into the +// style's language-independent rule and so reaches the other languages too; made on a box in +// another language it goes into that language's own rule only. Font is the deliberate +// exception: a font suits a script, so it is always per language. +// - Paragraph-tab and Highlighting-tab settings are per style, never per language. +// +// The dialog's dropdowns are select2 controls over hidden in the dialog. */ +interface ISelectOption { + value: string; + text: string; +} + +/** The options of one of the dialog's s. */ +async function openSelect2(frame: Frame, selectId: string): Promise { + // select2 hides the holds it. + * `wanted` describes the entry for the error when there is no such entry. + */ +async function chooseSelect2Option( + frame: Frame, + selectId: string, + matches: (option: ISelectOption) => boolean, + wanted: string, +): Promise { + const options = await getSelectOptions(frame, selectId); + const index = options.findIndex(matches); + if (index < 0) + throw new Error( + `The Format dialog offers no ${wanted}. It offers: ` + + options.map((o) => `"${o.text}"`).join(", ") + + ".", + ); + await openSelect2(frame, selectId); + // The dropdown lists the