Skip to content

Commit

Permalink
Improve Pontoon RTL editing (mozilla#2926)
Browse files Browse the repository at this point in the history
* Add dir=ltr and style=white-space:pre to syntax

* Limit dir=ltr to explicitly known node names

* Fix directionality in nested contexts (e.g. tag -> quoted value -> placeholder)

* Update CodeMirror dependencies

* Use new EditorView.bidiIsolatedRanges

* Refactor decorator plugin to iterate tree only once

* Update all JS CI actions to use Node.js 18

* Update CodeMirror dependencies again

* Drop "unicode-bidi: isolate" style as unnecessary given the dir attribute

* Fix padding in editor for left/right scrolling
  • Loading branch information
eemeli authored Oct 12, 2023
1 parent 6c59b64 commit 5b6af5f
Show file tree
Hide file tree
Showing 9 changed files with 311 additions and 133 deletions.
13 changes: 6 additions & 7 deletions .github/workflows/frontend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ jobs:
name: TypeScript
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with: { node-version: '16' }
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with: { node-version: '18' }
- name: Install dependencies
run: npm ci
- name: Check TypeScript
Expand All @@ -35,10 +35,9 @@ jobs:
jest:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: '14'
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with: { node-version: '18' }
- name: Install globals
run: npm install --global npm@8
- name: Install dependencies
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/js-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ jobs:
name: eslint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with: { node-version: '16' }
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with: { node-version: '18' }
- name: Install dependencies
run: npm ci
- name: eslint
Expand All @@ -44,9 +44,9 @@ jobs:
name: prettier
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with: { node-version: '16' }
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with: { node-version: '18' }
- name: Install dependencies
run: npm ci
- name: prettier
Expand Down
7 changes: 3 additions & 4 deletions .github/workflows/tag-admin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ jobs:
name: Test & build
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: '14'
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with: { node-version: '18' }
- name: Install globals
run: npm install --global npm@8
- name: Install dependencies
Expand Down
42 changes: 21 additions & 21 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions translate/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
"name": "translate",
"private": true,
"dependencies": {
"@codemirror/autocomplete": "^6.7.1",
"@codemirror/commands": "^6.2.4",
"@codemirror/language": "^6.7.0",
"@codemirror/autocomplete": "^6.9.1",
"@codemirror/commands": "^6.2.5",
"@codemirror/language": "^6.9.0",
"@codemirror/state": "^6.2.1",
"@codemirror/view": "^6.12.0",
"@codemirror/view": "^6.19.0",
"@fluent/bundle": "^0.18.0",
"@fluent/langneg": "^0.7.0",
"@fluent/react": "^0.15.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,13 @@
background: white;
color: var(--dark-grey-4);
font-size: 14px;
padding: 6px 8px 6px 4px;
padding: 6px 0;
}

.cm-editor .cm-scroller {
font-family: inherit;
line-height: 22px;
padding: 0 8px 0 4px;
}

.singlefield .cm-content,
Expand All @@ -102,6 +103,10 @@
padding: 0;
}

.translationform .cm-scroller {
padding: 0;
}

.translationform .cm-content {
padding: 2px 0;
}
Expand Down
142 changes: 142 additions & 0 deletions translate/src/modules/translationform/utils/decoratorPlugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import { syntaxTree } from '@codemirror/language';
import { Prec, RangeSetBuilder } from '@codemirror/state';
import {
Decoration,
DecorationSet,
Direction,
EditorView,
ViewPlugin,
ViewUpdate,
} from '@codemirror/view';
import type { Tree } from '@lezer/common';

/** Use content-based automatic direction for values inside quotes */
const dirAuto = Decoration.mark({ attributes: { dir: 'auto' } });

/** Explicitly mark placeholders and tags as LTR spans, for bidirectional contexts */
const dirLTR = Decoration.mark({
attributes: { dir: 'ltr' },
bidiIsolate: Direction.LTR,
});

/** Enable spellchecking only for string content, and not highlighted syntax or quoted literals */
const spellcheck = Decoration.mark({ attributes: { spellcheck: 'true' } });

/**
* Because decorators may be nested, they need to be tracked separately
* so that we can assign appropriate precedences to them later.
* In the worst case, we'll have a dir=LTR tag in a dir=RTL message
* containing a dir=auto quoted literal with a dir=LTR placeholder.
* Because placeholders may also contain quoted literals,
* the placeholders inside & outside literals need different precedence.
* Luckily no format we cares about allows for
* placeholders within quoted literals within placeholders.
*/
const getDecorations = (view: EditorView) => {
const phIn = new RangeSetBuilder<Decoration>(); // placeholders inside quotes
const lit = new RangeSetBuilder<Decoration>(); // quoted literals
const phOut = new RangeSetBuilder<Decoration>(); // placeholders outside quotes
const ts = new RangeSetBuilder<Decoration>(); // tags and spellcheck
let ph = phOut;
let quoteStart = -1;
let phStart = -1;
let tagStart = -1;
let end = -1;
syntaxTree(view.state).iterate({
enter(node) {
switch (node.name) {
case 'Document':
end = node.to;
break;
case 'keyword':
ph.add(node.from, node.to, dirLTR);
break;
case 'brace':
if (phStart === -1) {
phStart = node.from;
} else {
ph.add(phStart, node.to, dirLTR);
phStart = -1;
}
break;
case 'quote':
if (quoteStart === -1) {
ph = phIn;
quoteStart = node.to;
} else {
if (node.from > quoteStart) {
lit.add(quoteStart, node.from, dirAuto);
}
ph = phOut;
quoteStart = -1;
}
break;
case 'string':
ts.add(node.from, node.to, spellcheck);
break;
case 'bracket':
if (tagStart === -1) {
tagStart = node.from;
} else {
ts.add(tagStart, node.to, dirLTR);
tagStart = -1;
}
break;
}
},
});
if (phStart !== -1 && end > phStart) {
ph.add(phStart, end, dirLTR);
}
if (quoteStart !== -1 && end > quoteStart) {
lit.add(quoteStart, end, dirAuto);
}
if (tagStart !== -1 && end > tagStart) {
ts.add(tagStart, end, dirLTR);
}
return {
literals: lit.finish(),
placeholdersOutsideQuotes: phOut.finish(),
placeholdersInsideQuotes: phIn.finish(),
tagsAndSpellcheck: ts.finish(),
};
};

export const decoratorPlugin = ViewPlugin.fromClass(
class {
decorations: ReturnType<typeof getDecorations>;
tree: Tree;
constructor(view: EditorView) {
this.decorations = getDecorations(view);
this.tree = syntaxTree(view.state);
}
update(update: ViewUpdate) {
if (update.docChanged || syntaxTree(update.state) != this.tree) {
this.decorations = getDecorations(update.view);
this.tree = syntaxTree(update.state);
}
}
},
{
provide(plugin) {
const list = (
get: (deco: ReturnType<typeof getDecorations>) => DecorationSet,
) => {
const get_ = (view: EditorView) => {
const pi = view.plugin(plugin);
return pi ? get(pi.decorations) : Decoration.none;
};
return [
EditorView.decorations.of(get_),
EditorView.bidiIsolatedRanges.of(get_),
];
};
return [
Prec.high(list((deco) => deco.placeholdersInsideQuotes)),
Prec.default(list((deco) => deco.literals)),
Prec.low(list((deco) => deco.placeholdersOutsideQuotes)),
Prec.lowest(list((deco) => deco.tagsAndSpellcheck)),
];
},
},
);
Loading

0 comments on commit 5b6af5f

Please sign in to comment.