[Suspense] Use style.setProperty to set display (#15882)
Follow up to #15861. Turns out you can't set `!important` using a normal property assignment. You have to use `style.setProperty`. Maybe Andrew *should* just learn CSS. IE9 doesn't support `style.setProperty` so we'll fall back to setting `display: none` without `important`, like we did before #15861 Our advice for apps that need to support IE9 will be to avoid using `!important`. Which seems like good advice in general, but IANACSSE. Tested on FB and using our Suspense DOM fixture.
Andrew Clark committed
Jun 13, 2019 at 18:17 UTC
2fe8fd290b0c315d6a9af99f7a4e038a2026598a
3 files changed
+20
-15
packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js
+11
-11
@@ -83,25 +83,25 @@ describe('ReactDOMSuspensePlaceholder', () => {
83
<div ref={divs[1]}>
84
<AsyncText ms={500} text="B" />
85
</div>
86
- <div style={{display: 'block'}} ref={divs[2]}>
86
+ <div style={{display: 'inline'}} ref={divs[2]}>
87
<Text text="C" />
88
</div>
89
</Suspense>
90
);
91
}
92
ReactDOM.render(<App />, container);
93
- expect(divs[0].current.style.display).toEqual('none !important');
94
- expect(divs[1].current.style.display).toEqual('none !important');
95
- expect(divs[2].current.style.display).toEqual('none !important');
93
+ expect(window.getComputedStyle(divs[0].current).display).toEqual('none');
94
+ expect(window.getComputedStyle(divs[1].current).display).toEqual('none');
95
+ expect(window.getComputedStyle(divs[2].current).display).toEqual('none');
96
97
await advanceTimers(500);
98
99
Scheduler.flushAll();
100
101
- expect(divs[0].current.style.display).toEqual('');
102
- expect(divs[1].current.style.display).toEqual('');
101
+ expect(window.getComputedStyle(divs[0].current).display).toEqual('block');
102
+ expect(window.getComputedStyle(divs[1].current).display).toEqual('block');
103
// This div's display was set with a prop.
104
- expect(divs[2].current.style.display).toEqual('block');
104
+ expect(window.getComputedStyle(divs[2].current).display).toEqual('inline');
105
});
106
107
it('hides and unhides timed out text nodes', async () => {
@@ -156,14 +156,14 @@ describe('ReactDOMSuspensePlaceholder', () => {
156
ReactDOM.render(<App />, container);
157
});
158
expect(container.innerHTML).toEqual(
159
- '<span style="display: none !important;">Sibling</span><span style=' +
160
- '"display: none !important;"></span>Loading...',
159
+ '<span style="display: none;">Sibling</span><span style=' +
160
+ '"display: none;"></span>Loading...',
161
);
162
163
act(() => setIsVisible(true));
164
expect(container.innerHTML).toEqual(
165
- '<span style="display: none !important;">Sibling</span><span style=' +
166
- '"display: none !important;"></span>Loading...',
165
+ '<span style="display: none;">Sibling</span><span style=' +
166
+ '"display: none;"></span>Loading...',
167
);
168
169
await advanceTimers(500);
packages/react-dom/src/client/ReactDOMHostConfig.js
+6
-1
@@ -589,7 +589,12 @@ export function hideInstance(instance: Instance): void {
589
// TODO: Does this work for all element types? What about MathML? Should we
590
// pass host context to this method?
591
instance = ((instance: any): HTMLElement);
592
- instance.style.display = 'none !important';
592
+ const style = instance.style;
593
+ if (typeof style.setProperty === 'function') {
594
+ style.setProperty('display', 'none', 'important');
595
+ } else {
596
+ style.display = 'none';
597
+ }
598
}
599
600
export function hideTextInstance(textInstance: TextInstance): void {
packages/react-fresh/src/__tests__/ReactFresh-test.js
+3
-3
@@ -1359,7 +1359,7 @@ describe('ReactFresh', () => {
1359
const fallbackChild = container.childNodes[1];
1360
expect(primaryChild.textContent).toBe('Content 1');
1361
expect(primaryChild.style.color).toBe('green');
1362
- expect(primaryChild.style.display).toBe('none !important');
1362
+ expect(primaryChild.style.display).toBe('none');
1363
expect(fallbackChild.textContent).toBe('Fallback 0');
1364
expect(fallbackChild.style.color).toBe('green');
1365
expect(fallbackChild.style.display).toBe('');
@@ -1373,7 +1373,7 @@ describe('ReactFresh', () => {
1373
expect(container.childNodes[1]).toBe(fallbackChild);
1374
expect(primaryChild.textContent).toBe('Content 1');
1375
expect(primaryChild.style.color).toBe('green');
1376
- expect(primaryChild.style.display).toBe('none !important');
1376
+ expect(primaryChild.style.display).toBe('none');
1377
expect(fallbackChild.textContent).toBe('Fallback 1');
1378
expect(fallbackChild.style.color).toBe('green');
1379
expect(fallbackChild.style.display).toBe('');
@@ -1397,7 +1397,7 @@ describe('ReactFresh', () => {
1397
expect(container.childNodes[1]).toBe(fallbackChild);
1398
expect(primaryChild.textContent).toBe('Content 1');
1399
expect(primaryChild.style.color).toBe('red');
1400
- expect(primaryChild.style.display).toBe('none !important');
1400
+ expect(primaryChild.style.display).toBe('none');
1401
expect(fallbackChild.textContent).toBe('Fallback 1');
1402
expect(fallbackChild.style.color).toBe('red');
1403
expect(fallbackChild.style.display).toBe('');