Skip to content

impr: enable dots typed effect for ligature languages (@byseif21) #7458

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
byseif21 wants to merge 14 commits into
base: master
Choose a base branch
from
+102 −28
Open

impr: enable dots typed effect for ligature languages (@byseif21) #7458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c1bdd6c
impr: enable dots typed effect for ligature languages (@byseif21)
byseif21 Jan 28, 2026
a6cddd2
readd if active
byseif21 Jan 28, 2026
e830566
refactor abit for readability
byseif21 Jan 28, 2026
709adbb
yap
byseif21 Jan 28, 2026
b539944
fix moving words
byseif21 Jan 28, 2026
8caa019
fix for blind
byseif21 Jan 28, 2026
eed55cb
dont center
byseif21 Jan 28, 2026
e051784
optmize
byseif21 Jan 28, 2026
32694a1
fix changing effect mid test after refresh
byseif21 Jan 28, 2026
aae08c9
fix resizing mid test
byseif21 Jan 28, 2026
6760d2b
use screenBounds
byseif21 Jan 28, 2026
b2d8d2c
.
byseif21 Jan 28, 2026
5b72288
limit to ligature update only
byseif21 Jan 28, 2026
e6cf966
move
byseif21 Jan 28, 2026
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
76 changes: 49 additions & 27 deletions frontend/src/styles/test.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@
&.blind {
.word {
& letter.extra {
display: none;
display: none !important;
}
& letter.incorrect {
color: var(--correct-letter-color);
Expand Down Expand Up @@ -533,45 +533,67 @@
}
}

&.typed-effect-dots:not(.withLigatures) {
&.typed-effect-dots {
/* transform already typed letters into appropriately colored dots */

.word letter {
position: relative;
&::after {
content: "";
position: absolute;
top: 50%;
left: 50%;
transform: translate(-50%, -50%);
width: 1em;
aspect-ratio: 1;
border-radius: 50%;
opacity: 0;
&:not(.withLigatures) .word,
&.withLigatures .word.broken-ligatures {
letter {
position: relative;
display: inline-block;
&::after {
content: "";
position: absolute;
top: 50%;
left: 50%;
transform: translate(-50%, -50%);
width: 1em;
aspect-ratio: 1;
border-radius: 50%;
opacity: 0;
}
}
}
.typed letter {
color: var(--bg-color);
animation: typedEffectToDust 200ms ease-out 0ms 1 forwards !important;
&::after {
animation: typedEffectFadeIn 100ms ease-in 100ms 1 forwards;
background: var(--c-dot);
// unify dote spaceing
&.withLigatures .word.broken-ligatures {
display: inline-flex;
letter {
width: 0.5em;
}
}
&:not(.blind) {

&:not(.withLigatures) .typed,
&.withLigatures .word.broken-ligatures.typed {
letter {
color: var(--bg-color);
animation: typedEffectToDust 200ms ease-out 0ms 1 forwards !important;
&::after {
animation: typedEffectFadeIn 100ms ease-in 100ms 1 forwards;
background: var(--c-dot);
}
}
}

&:not(.withLigatures):not(.blind) {
.word letter.incorrect::after {
background: var(--c-dot--error);
}
}
&.withLigatures:not(.blind) .word.broken-ligatures letter.incorrect::after {
background: var(--c-dot--error);
}

@media (prefers-reduced-motion) {
.typed letter {
animation: none !important;
transform: scale(0.4);
color: transparent;
&::after {
&:not(.withLigatures) .typed,
&.withLigatures .word.broken-ligatures.typed {
letter {
animation: none !important;
opacity: 1;
transform: scale(0.4);
color: transparent;
&::after {
animation: none !important;
opacity: 1;
}
}
}
}
Expand Down
45 changes: 45 additions & 0 deletions frontend/src/ts/test/break-ligatures.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import Config from "../config";
import { ElementWithUtils } from "../utils/dom";

function canBreak(wordEl: ElementWithUtils): boolean {
if (Config.typedEffect !== "dots") return false;
if (wordEl.hasClass("broken-ligatures")) return false;

return !!wordEl.native.closest(".withLigatures");
Copy link
Contributor

@Leonabcd123 Leonabcd123 Jan 28, 2026

Choose a reason for hiding this comment

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

Consider replacing with this:

return wordEl.getParent().hasClass("withLigatures");

Copy link
Contributor Author

@byseif21 byseif21 Jan 28, 2026

Choose a reason for hiding this comment

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

why? closest faster and better, getParent() creates a new ElementWithUtils object wrapper every single time

Copy link
Contributor

@Leonabcd123 Leonabcd123 Jan 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

closest seems less readable, as it doesn't make it clear what element might have the withLigatures class. Also (it might just be me), but I really hate the !! syntax.

Copy link
Contributor Author

@byseif21 byseif21 Jan 28, 2026

Choose a reason for hiding this comment

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

let's do the spin

Copy link
Contributor

@Leonabcd123 Leonabcd123 Jan 28, 2026

Choose a reason for hiding this comment

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

I lost, so keep current approach.

}

function applyIfNeeded(wordEl: ElementWithUtils): void {
if (!canBreak(wordEl)) return;

const { width } = wordEl.screenBounds();
wordEl.setStyle({ width: `${width}px` });
wordEl.addClass("broken-ligatures");
}

function reset(wordEl: ElementWithUtils): void {
if (!wordEl.hasClass("broken-ligatures")) return;
wordEl.removeClass("broken-ligatures");
wordEl.setStyle({ width: "" });
}

export function set(
wordEl: ElementWithUtils,
areLigaturesBroken: boolean,
): void {
areLigaturesBroken ? applyIfNeeded(wordEl) : reset(wordEl);
}

export function update(key: string, wordsEl: ElementWithUtils): void {
const words = wordsEl.qsa(".word.typed");

const shouldReset =
!wordsEl.hasClass("withLigatures") ||
Config.typedEffect !== "dots" ||
key === "fontFamily" ||
key === "fontSize";

if (shouldReset) {
words.forEach(reset);
}
words.forEach(applyIfNeeded);
Copy link
Contributor Author

@byseif21 byseif21 Jan 28, 2026

Choose a reason for hiding this comment

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

check if ok? or heavy and should we do batching instead?

Copy link
Contributor

@Leonabcd123 Leonabcd123 Jan 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I'd do this:

words.forEach((word) => {
  if (shouldReset) reset(word);
  applyIfNeeded(word);
})

(not tested)

Copy link
Contributor Author

@byseif21 byseif21 Jan 28, 2026

Choose a reason for hiding this comment

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

i meant performance wise

Copy link
Contributor

@Leonabcd123 Leonabcd123 Jan 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Performance wise we're running on the words twice when shouldReset is true, while we can do it one pass. I don't see why go with this approach rather than doing it in one loop.

Copy link
Contributor Author

@byseif21 byseif21 Jan 28, 2026

Choose a reason for hiding this comment

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

what do you think about batching then? too much or worth it?

Copy link
Contributor

@Leonabcd123 Leonabcd123 Jan 28, 2026

Choose a reason for hiding this comment

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

I'd say it's worth it.

}
9 changes: 8 additions & 1 deletion frontend/src/ts/test/test-ui.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import * as SlowTimer from "../states/slow-timer";
import * as TestConfig from "./test-config";
import * as CompositionDisplay from "../elements/composition-display";
import * as AdController from "../controllers/ad-controller";
import * as Ligatures from "./break-ligatures";
import * as LayoutfluidFunboxTimer from "../test/funbox/layoutfluid-funbox-timer";
import * as Keymap from "../elements/keymap";
import * as ThemeController from "../controllers/theme-controller";
Expand Down Expand Up @@ -141,6 +142,7 @@ export function updateActiveElement(
if (previousActiveWord !== null) {
if (direction === "forward") {
previousActiveWord.addClass("typed");
Ligatures.set(previousActiveWord, true);
} else if (direction === "back") {
//
}
Expand All @@ -157,6 +159,7 @@ export function updateActiveElement(
newActiveWord.addClass("active");
newActiveWord.removeClass("error");
newActiveWord.removeClass("typed");
Ligatures.set(newActiveWord, false);

activeWordTop = newActiveWord.getOffsetTop();
activeWordHeight = newActiveWord.getOffsetHeight();
Expand Down Expand Up @@ -2072,11 +2075,15 @@ ConfigEvent.subscribe(({ key, newValue }) => {
"colorfulMode",
"showAllLines",
"fontSize",
"fontFamily",
Copy link
Contributor

@Leonabcd123 Leonabcd123 Jan 28, 2026

Choose a reason for hiding this comment

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

We now run updateWordWrapperClasses unnecessarily when a user changes fontFamily.

Copy link
Contributor Author

@byseif21 byseif21 Jan 28, 2026

Choose a reason for hiding this comment

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

the thing is I think I still need some of the layout updates it triggers when fontFamily changes. I’ll give it some manual testing to see

Copy link
Contributor

@Leonabcd123 Leonabcd123 Jan 28, 2026

Choose a reason for hiding this comment

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

Can we do this?

if (key !== "fontFamily"){
  updateWordWrapperClasses();
}
Ligatures.update(key, wordsEl);

Copy link
Contributor Author

@byseif21 byseif21 Jan 28, 2026

Choose a reason for hiding this comment

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

kinda feel like fontFamily should also trigger updateWordWrapperClasses but can’t really prove it visually enough till now, so yeah ok I’ll leave it limited for now

"maxLineWidth",
"tapeMargin",
].includes(key)
) {
updateWordWrapperClasses();
if (key !== "fontFamily") updateWordWrapperClasses();
if (["typedEffect", "fontFamily", "fontSize"].includes(key)) {
Ligatures.update(key, wordsEl);
}
}
if (["tapeMode", "tapeMargin"].includes(key)) {
updateLiveStatsMargin();
Expand Down