Skip to content

Add e2e test for All Format Controls (Notion test case 357); Format text color reaches all languages (BL-16803) #8288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JohnThomson wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add e2e test for All Format Controls (Notion test case 357); Format text color reaches all languages (BL-16803) #8288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 72 additions & 6 deletions src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<string, string> {
const fonts = new Map<string, string>();
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();
Expand Down Expand Up @@ -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);
}

Expand Down
205 changes: 205 additions & 0 deletions src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditorSpec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
"<div id='testTarget' class='foo-style' lang='xyz'></div>" +
"<div id='colorSelectButton'></div>",
);
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(
"<div id='testTarget' class='foo-style' lang='abc'></div>" +
"<div id='colorSelectButton'></div>",
);
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 =
"<select id='size-select'><option selected>12</option></select>" +
"<select id='line-height-select'><option selected>1.5</option></select>" +
"<select id='word-space-select'><option selected>Normal</option><option>Wide</option><option>Extra Wide</option></select>" +
"<select id='para-spacing-select'><option selected>0</option></select>" +
"<div id='bold'></div><div id='italic'></div><div id='underline'></div>" +
"<div id='indent-none' class='selectedIcon'></div><div id='position-left' class='selectedIcon'></div>" +
"<div id='colorSelectButton'></div>" +
"<select id='styleSelect'></select>" +
"<div id='style-group' class='state-enteringStyle'></div>" +
"<input id='style-select-input' value='Bar'>";

// 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(
"<div id='testTarget' class='foo-style' lang='xyz'></div>" +
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(
"<div id='testTarget' class='foo-style' lang='xyz'></div>" +
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(
"<div class='bloom-translationGroup foo-style'>" +
"<div id='testTarget' class='bloom-editable foo-style' lang='xyz'></div>" +
"<div id='sibling' class='bloom-editable foo-style' lang='abc'></div>" +
"</div>" +
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(
"<div id='testTarget' class='foo-style' lang='xyz'></div>",
Expand Down
27 changes: 20 additions & 7 deletions src/BloomE2E/helpers/bookMaking.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,17 +521,21 @@ export async function goToPage(page: Page, pageId: string): Promise<void> {
/**
* 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).
*/
export async function clickInGroup(
page: Page,
groupSelector: string,
languageTag: string,
groupIndex = 0,
): Promise<Locator> {
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();
Expand All @@ -544,26 +548,35 @@ 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(
page: Page,
groupSelector: string,
languageTag: string,
text: string,
groupIndex = 0,
): Promise<void> {
// 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. */
Expand Down
Loading