Skip to content

Commit 878fc60

Browse files
authored
Choices text box: make sure all choice tokens are readable (#1982)
## Context This is a follow-up to #1529, where we made sure that choice tokens with custom bg color but no custom fg color would not become unreadable when switching from light to dark themes. The tokens that were rendered when editing a text cell did not benefit from the previous fix. So while the tokens stayed readable when looking at them, they could go back to being unreadable when editing them after double clicking a cell (with automatic light text in dark theme, on a light custom bg, for example). ## Proposed solution Make sure the choices text box also applies the "readable colors combo" calculation. ## Related issues #1155, see this comment: #1155 (comment) ## Has this been tested? - [ ] 👍 yes, I added tests to the test suite - [ ] 💭 no, because this PR is a draft and still needs work - [x] 🙅 no, because this is not relevant here - [ ] 🙋 no, because I need help <!-- Detail how we can help you --> ## Screenshots / Screencasts Before: https://github.com/user-attachments/assets/2d87d047-3678-4e39-a079-7fb98c9142ae After: https://github.com/user-attachments/assets/732bd4dc-b041-4caf-b2bb-8f227c8bd3e3 Sorry it took so long @S7venLights 😬
1 parent 8568497 commit 878fc60

File tree

3 files changed

+24
-25
lines changed

3 files changed

+24
-25
lines changed

app/client/widgets/ChoiceListEditor.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ import {CellValue} from "app/common/DocActions";
1515
import {CompiledPredicateFormula} from 'app/common/PredicateFormula';
1616
import {EmptyRecordView} from 'app/common/RecordView';
1717
import {decodeObject, encodeObject} from 'app/plugin/objtypes';
18-
import {ChoiceOptions, getRenderFillColor, getRenderTextColor} from 'app/client/widgets/ChoiceTextBox';
19-
import {choiceToken, cssChoiceACItem, cssChoiceToken} from 'app/client/widgets/ChoiceToken';
18+
import {ChoiceOptions} from 'app/client/widgets/ChoiceTextBox';
19+
import {choiceToken, choiceTokenDomArgs, cssChoiceACItem} from 'app/client/widgets/ChoiceToken';
2020
import {icon} from 'app/client/ui2018/icons';
2121
import {dom, styled} from 'grainjs';
2222

@@ -95,17 +95,14 @@ export class ChoiceListEditor extends NewBaseEditor {
9595

9696
this._tokenField = TokenField.ctor<ChoiceItem>().create(this, {
9797
initialValue: startTokens,
98-
renderToken: item => [
98+
renderToken: item => choiceTokenDomArgs(
9999
item.isBlank ? '[Blank]' : item.label,
100-
dom.style('background-color', getRenderFillColor(this._choiceOptionsByName[item.label])),
101-
dom.style('color', getRenderTextColor(this._choiceOptionsByName[item.label])),
102-
dom.cls('font-bold', this._choiceOptionsByName[item.label]?.fontBold ?? false),
103-
dom.cls('font-underline', this._choiceOptionsByName[item.label]?.fontUnderline ?? false),
104-
dom.cls('font-italic', this._choiceOptionsByName[item.label]?.fontItalic ?? false),
105-
dom.cls('font-strikethrough', this._choiceOptionsByName[item.label]?.fontStrikethrough ?? false),
106-
cssChoiceToken.cls('-invalid', item.isInvalid),
107-
cssChoiceToken.cls('-blank', item.isBlank),
108-
],
100+
{
101+
...(this._choiceOptionsByName[item.label] || {}),
102+
invalid: item.isInvalid,
103+
blank: item.isBlank,
104+
},
105+
),
109106
createToken: label => new ChoiceItem(label, !this._choicesSet.has(label), label.trim() === ''),
110107
acOptions,
111108
openAutocompleteOnFocus: true,

app/client/widgets/ChoiceTextBox.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {cssLabel, cssRow} from 'app/client/ui/RightPanelStyles';
1414
import {testId, theme} from 'app/client/ui2018/cssVars';
1515
import {icon} from 'app/client/ui2018/icons';
1616
import {ChoiceListEntry} from 'app/client/widgets/ChoiceListEntry';
17-
import {choiceToken, DEFAULT_BACKGROUND_COLOR, DEFAULT_COLOR} from 'app/client/widgets/ChoiceToken';
17+
import {choiceToken} from 'app/client/widgets/ChoiceToken';
1818
import {NTextBox} from 'app/client/widgets/NTextBox';
1919
import {Computed, dom, styled} from 'grainjs';
2020

@@ -24,14 +24,6 @@ export type ChoiceOptionsByName = Map<string, IChoiceOptions | undefined>;
2424

2525
const t = makeT('ChoiceTextBox');
2626

27-
export function getRenderFillColor(choiceOptions?: IChoiceOptions) {
28-
return choiceOptions?.fillColor ?? DEFAULT_BACKGROUND_COLOR;
29-
}
30-
31-
export function getRenderTextColor(choiceOptions?: IChoiceOptions) {
32-
return choiceOptions?.textColor ?? DEFAULT_COLOR;
33-
}
34-
3527
/**
3628
* ChoiceTextBox - A textbox for choice values.
3729
*/

app/client/widgets/ChoiceToken.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,21 @@ export function choiceToken(
3434
options: IChoiceTokenOptions,
3535
...args: DomElementArg[]
3636
): DomContents {
37+
return cssChoiceToken(choiceTokenDomArgs(label, options), ...args);
38+
}
39+
40+
/**
41+
* Exposes the choiceToken dom args outside of cssChoiceToken to allow
42+
* easy usage of them with TokenField#renderToken, that has its own wrapper dom el.
43+
*/
44+
export function choiceTokenDomArgs(
45+
label: DomElementArg,
46+
options: IChoiceTokenOptions,
47+
): DomElementArg {
3748
const {fillColor, textColor, fontBold, fontItalic, fontUnderline,
38-
fontStrikethrough, invalid, blank} = options;
49+
fontStrikethrough, invalid, blank} = options;
3950
const {bg, fg} = getReadableColorsCombo({fillColor, textColor});
40-
return cssChoiceToken(
51+
return [
4152
label,
4253
dom.style('background-color', bg),
4354
dom.style('color', fg),
@@ -47,8 +58,7 @@ export function choiceToken(
4758
dom.cls('font-strikethrough', fontStrikethrough ?? false),
4859
invalid ? cssChoiceToken.cls('-invalid') : null,
4960
blank ? cssChoiceToken.cls('-blank') : null,
50-
...args
51-
);
61+
];
5262
}
5363

5464
export const cssChoiceToken = styled('div', `

0 commit comments

Comments
 (0)