Skip to content

Commit 4f9b407

Browse files
authored
Fix/issue 64 promise false class (#68)
* Fix #64: Handle promises resolving to false in ClassObjectSink * Address PR review feedback: restructure tests and simplify asap usage
1 parent 5b94390 commit 4f9b407

2 files changed

Lines changed: 86 additions & 24 deletions

File tree

src/sinks/class-sink.test.ts

Lines changed: 80 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -30,35 +30,92 @@ describe('Class Sink', () => {
3030

3131
describe('Given a class object', () => {
3232

33-
it('sets classes for truthy attributes on sink', () => {
34-
const el = MockElement();
35-
const sink = ClassObjectSink(<HTMLElement>el);
33+
describe('when a property of the object is a present value', () => {
34+
35+
it('sets classes for truthy attributes on sink', () => {
36+
const el = MockElement();
37+
const sink = ClassObjectSink(<HTMLElement>el);
38+
39+
sink({
40+
class1: true,
41+
class2: 1,
42+
class3: 'yes!',
43+
});
44+
expect(el.className).toContain('class1');
45+
expect(el.className).toContain('class2');
46+
expect(el.className).toContain('class3');
47+
});
3648

37-
sink({
38-
class1: true,
39-
class2: 1,
40-
class3: 'yes!',
49+
it('clears classes for falsy attributes on sink', () => {
50+
const el = MockElement({ className: 'class1 class2 class3' });
51+
const sink = ClassObjectSink(<HTMLElement>el);
52+
expect(el.className).toContain('class1');
53+
expect(el.className).toContain('class2');
54+
expect(el.className).toContain('class3');
55+
56+
sink({
57+
class1: false,
58+
class2: 0,
59+
class3: '',
60+
});
61+
expect(el.className).not.toContain('class1');
62+
expect(el.className).not.toContain('class2');
63+
expect(el.className).not.toContain('class3');
4164
});
42-
expect(el.className).toContain('class1');
43-
expect(el.className).toContain('class2');
44-
expect(el.className).toContain('class3');
65+
66+
it('should not add class when value is false', () => {
67+
const el = MockElement();
68+
const sink = ClassObjectSink(<HTMLElement>el);
69+
70+
sink({
71+
dotted: false,
72+
});
73+
74+
expect(el.className).not.toContain('dotted');
75+
expect(el.className).toEqual('');
76+
});
77+
4578
});
4679

47-
it('clears classes for falsy attributes on sink', () => {
48-
const el = MockElement({ className: 'class1 class2 class3' });
49-
const sink = ClassObjectSink(<HTMLElement>el);
50-
expect(el.className).toContain('class1');
51-
expect(el.className).toContain('class2');
52-
expect(el.className).toContain('class3');
80+
describe('when a property of the object is a future value', () => {
81+
82+
describe('when it resolves/emits false', () => {
83+
84+
it('should not add a class corresponding to the property name', async () => {
85+
const el = MockElement();
86+
const sink = ClassObjectSink(<HTMLElement>el);
87+
88+
const dottedPromise = Promise.resolve(false);
89+
sink({
90+
dotted: dottedPromise,
91+
});
92+
93+
await dottedPromise;
94+
95+
expect(el.className).not.toContain('dotted');
96+
expect(el.className).toEqual('');
97+
});
5398

54-
sink({
55-
class1: false,
56-
class2: 0,
57-
class3: '',
5899
});
59-
expect(el.className).not.toContain('class1');
60-
expect(el.className).not.toContain('class2');
61-
expect(el.className).not.toContain('class3');
100+
101+
describe('when it resolves/emits true', () => {
102+
103+
it('should add a class corresponding to the property name', async () => {
104+
const el = MockElement();
105+
const sink = ClassObjectSink(<HTMLElement>el);
106+
107+
const activePromise = Promise.resolve(true);
108+
sink({
109+
active: activePromise,
110+
});
111+
112+
await activePromise;
113+
114+
expect(el.className).toContain('active');
115+
});
116+
117+
});
118+
62119
});
63120

64121
});

src/sinks/class-sink.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,12 @@ export const ClassObjectSink: Sink<Element> = (node: Element) => {
3535
// FIXME: is it safe to assume it's an object, at this point?
3636
: (<(ClassName | ClassRecord)[]>[]).concat(name).forEach(obj => Object.entries(obj)
3737
// TODO: support 3-state with toggle
38-
.forEach(([k, v]) => asap(v ? add : remove, k))
38+
.forEach(([k, v]) => {
39+
// Use asap to handle both present and future values
40+
// For futures (Promise/Observable), it will wait for resolution
41+
// For present values, it will execute immediately
42+
asap((resolvedValue: any) => resolvedValue ? add(k) : remove(k), v);
43+
})
3944
)
4045
};
4146
};

0 commit comments

Comments
 (0)