Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Commit 2327c9c

Browse files
author
Nathan Sobo
authored
Merge pull request #1093 from atom/ns/fix-flakys
Overhaul height invalidation
2 parents 75dcfbe + 694bcb0 commit 2327c9c

File tree

6 files changed

+36
-89
lines changed

6 files changed

+36
-89
lines changed

‎lib/project/list-view.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
const etch = require('etch');
22
const $ = etch.dom;
3-
const resizeDetector = require('element-resize-detector');
43

54
module.exports = class ListView {
65
constructor({items, heightForItem, itemComponent, className}) {
@@ -12,7 +11,8 @@ module.exports = class ListView {
1211
this.previousClientHeight = 0
1312
etch.initialize(this);
1413

15-
resizeDetector({strategy: 'scroll'}).listenTo(this.element, () => etch.update(this));
14+
const resizeObserver = new ResizeObserver(() => etch.update(this));
15+
resizeObserver.observe(this.element);
1616
this.element.addEventListener('scroll', () => etch.update(this));
1717
}
1818

‎lib/project/results-view.js‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ const {
1010

1111
const ListView = require('./list-view');
1212
const etch = require('etch');
13-
const resizeDetector = require('element-resize-detector');
1413
const binarySearch = require('binary-search')
1514

1615
const path = require('path');
@@ -31,9 +30,6 @@ class ResultsView {
3130
this.model = model;
3231
this.pixelOverdraw = 100;
3332

34-
this.resolveHeightInvalidationPromise = null
35-
this.heightInvalidationPromise = new Promise((resolve) => { this.resolveHeightInvalidationPromise = resolve })
36-
3733
this.resultRowGroups = Object.values(model.results).map(result =>
3834
new ResultRowGroup(result, this.model.getFindOptions())
3935
)
@@ -63,10 +59,8 @@ class ResultsView {
6359

6460
etch.initialize(this);
6561

66-
resizeDetector({strategy: 'scroll'}).listenTo(
67-
this.element,
68-
this.invalidateItemHeights.bind(this)
69-
);
62+
const resizeObserver = new ResizeObserver(this.invalidateItemHeights.bind(this));
63+
resizeObserver.observe(this.element);
7064
this.element.addEventListener('mousedown', this.handleClick.bind(this));
7165

7266
this.subscriptions = new CompositeDisposable(
@@ -190,7 +184,6 @@ class ResultsView {
190184
this.contextRowHeight = contextRowHeight;
191185
this.clientHeight = clientHeight;
192186
await etch.update(this);
193-
this.resolveHeightInvalidationPromise();
194187
}
195188

196189
etch.update(this);

‎package-lock.json‎

Lines changed: 0 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
},
3333
"dependencies": {
3434
"binary-search": "^1.3.3",
35-
"element-resize-detector": "^1.1.10",
3635
"etch": "0.9.3",
3736
"fs-plus": "^3.0.0",
3837
"temp": "^0.8.3",

‎spec/project-find-view-spec.js‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ describe(`ProjectFindView (ripgrep=${ripgrep})`, () => {
2222
function getExistingResultsPane() {
2323
const pane = atom.workspace.paneForURI(ResultsPaneView.URI);
2424
if (pane) {
25-
26-
// Allow element-resize-detector to perform batched measurements
27-
advanceClock(1);
28-
2925
return pane.itemForURI(ResultsPaneView.URI);
3026
}
3127
}
@@ -303,7 +299,6 @@ describe(`ProjectFindView (ripgrep=${ripgrep})`, () => {
303299
await searchPromise;
304300

305301
const resultsView = getResultsView();
306-
await resultsView.heightInvalidationPromise
307302
expect(resultsView.element).toBeVisible();
308303
expect(resultsView.refs.listView.element.querySelectorAll(".match-row")).toHaveLength(2);
309304
})
@@ -317,7 +312,6 @@ describe(`ProjectFindView (ripgrep=${ripgrep})`, () => {
317312
await searchPromise;
318313

319314
const resultsView = getResultsView();
320-
await resultsView.heightInvalidationPromise
321315
expect(resultsView.element).toBeVisible();
322316
expect(resultsView.refs.listView.element.querySelectorAll(".match-row")).toHaveLength(1);
323317
});
@@ -329,7 +323,6 @@ describe(`ProjectFindView (ripgrep=${ripgrep})`, () => {
329323
await searchPromise;
330324

331325
const resultsView = getResultsView();
332-
await resultsView.heightInvalidationPromise
333326
expect(resultsView.element).toBeVisible();
334327
expect(resultsView.refs.listView.element.querySelectorAll(".match-row")).toHaveLength(2);
335328
expect(resultsView.refs.listView.element.querySelectorAll(".match.highlight-info")).toHaveLength(3);
@@ -342,7 +335,6 @@ describe(`ProjectFindView (ripgrep=${ripgrep})`, () => {
342335
await searchPromise;
343336

344337
const resultsView = getResultsView();
345-
await resultsView.heightInvalidationPromise
346338
expect(resultsView.element).toBeVisible();
347339
expect(resultsView.refs.listView.element.querySelectorAll(".match-row")).toHaveLength(1);
348340
});
@@ -714,7 +706,6 @@ describe(`ProjectFindView (ripgrep=${ripgrep})`, () => {
714706
await waitForSearchResults();
715707

716708
const resultsView = getResultsView();
717-
await resultsView.heightInvalidationPromise
718709
expect(resultsView.element).toBeVisible();
719710
expect(resultsView.refs.listView.element.querySelectorAll(".match-row")).toHaveLength(11);
720711
expect(resultsView.refs.listView.element.querySelectorAll(".match.highlight-info")).toHaveLength(13);
@@ -776,7 +767,6 @@ describe(`ProjectFindView (ripgrep=${ripgrep})`, () => {
776767
const resultsView = getResultsView();
777768
const resultsPaneView = getExistingResultsPane();
778769

779-
await resultsView.heightInvalidationPromise
780770
expect(resultsView.element).toBeVisible();
781771
expect(resultsView.refs.listView.element.querySelectorAll(".match-row")).toHaveLength(11);
782772
expect(resultsView.refs.listView.element.querySelectorAll(".match.highlight-info")).toHaveLength(13);
@@ -805,7 +795,6 @@ describe(`ProjectFindView (ripgrep=${ripgrep})`, () => {
805795
const listView = resultsView.refs.listView;
806796
const resultsPaneView = getExistingResultsPane();
807797

808-
await resultsView.heightInvalidationPromise
809798
expect(listView.element.querySelectorAll(".match-row")).toHaveLength(11);
810799
expect(listView.element.querySelectorAll(".match.highlight-info")).toHaveLength(13);
811800
expect(resultsPaneView.refs.previewCount.textContent).toBe("13 results found in 2 files for items");
@@ -851,7 +840,6 @@ describe(`ProjectFindView (ripgrep=${ripgrep})`, () => {
851840
const resultsView = getResultsView();
852841
const resultsPaneView = getExistingResultsPane();
853842

854-
await resultsView.heightInvalidationPromise
855843
expect(resultsView.refs.listView.element.querySelectorAll(".list-item")).toHaveLength(13);
856844
expect(resultsPaneView.refs.previewCount.textContent).toBe("13 results found in 2 files for items");
857845

‎spec/results-view-spec.js‎

Lines changed: 32 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -24,48 +24,48 @@ describe('ResultsView', () => {
2424

2525
function getResultsPane() {
2626
let pane = atom.workspace.paneForURI(ResultsPaneView.URI);
27-
28-
// Allow element-resize-detector to perform batched measurements
29-
advanceClock(1);
30-
3127
if (pane) return pane.itemForURI(ResultsPaneView.URI);
3228
}
3329

3430
function getResultsView() {
3531
return getResultsPane().refs.resultsView;
3632
}
3733

38-
function buildResultsView() {
34+
function buildResultsView(options = {}) {
3935
const FindOptions = require("../lib/find-options")
4036
const ResultsModel = require("../lib/project/results-model")
4137
const { Result } = ResultsModel
4238
const ResultsView = require("../lib/project/results-view")
4339
const model = new ResultsModel(new FindOptions({}), null)
4440
const resultsView = new ResultsView({ model })
45-
model.addResult("/a/b.txt", Result.create({
46-
filePath: "/a/b.txt",
47-
matches: [
48-
{
49-
lineText: "hello world",
50-
matchText: "world",
51-
range: {start: {row: 0, column: 6}, end: {row: 0, column: 11}},
52-
leadingContextLines: [],
53-
trailingContextLines: []
54-
}
55-
]
56-
}))
57-
model.addResult("/c/d.txt", Result.create({
58-
filePath: "/c/d.txt",
59-
matches: [
60-
{
61-
lineText: "goodnight moon",
62-
matchText: "night",
63-
range: {start: {row: 0, column: 4}, end: {row: 0, column: 8}},
64-
leadingContextLines: [],
65-
trailingContextLines: []
66-
}
67-
]
68-
}))
41+
42+
if (!options.empty) {
43+
model.addResult("/a/b.txt", Result.create({
44+
filePath: "/a/b.txt",
45+
matches: [
46+
{
47+
lineText: "hello world",
48+
matchText: "world",
49+
range: {start: {row: 0, column: 6}, end: {row: 0, column: 11}},
50+
leadingContextLines: [],
51+
trailingContextLines: []
52+
}
53+
]
54+
}))
55+
model.addResult("/c/d.txt", Result.create({
56+
filePath: "/c/d.txt",
57+
matches: [
58+
{
59+
lineText: "goodnight moon",
60+
matchText: "night",
61+
range: {start: {row: 0, column: 4}, end: {row: 0, column: 8}},
62+
leadingContextLines: [],
63+
trailingContextLines: []
64+
}
65+
]
66+
}))
67+
}
68+
6969
return resultsView
7070
}
7171

@@ -97,7 +97,6 @@ describe('ResultsView', () => {
9797
await searchPromise;
9898

9999
resultsView = getResultsView();
100-
await resultsView.heightInvalidationPromise;
101100
expect(resultsView.refs.listView.element.querySelector('.path-name').textContent).toBe("one-long-line.coffee");
102101
expect(resultsView.refs.listView.element.querySelectorAll('.preview').length).toBe(1);
103102
expect(resultsView.refs.listView.element.querySelector('.preview').textContent).toBe('test test test test test test test test test test test a b c d e f g h i j k l abcdefghijklmnopqrstuvwxyz');
@@ -116,7 +115,6 @@ describe('ResultsView', () => {
116115
await searchPromise;
117116

118117
resultsView = getResultsView();
119-
await resultsView.heightInvalidationPromise;
120118
expect(resultsView.refs.listView.element.querySelector('.path-name').textContent).toBe(path.join("project", "one-long-line.coffee"));
121119
});
122120
});
@@ -135,7 +133,6 @@ describe('ResultsView', () => {
135133
await searchPromise;
136134

137135
resultsView = getResultsView();
138-
await resultsView.heightInvalidationPromise;
139136
expect(resultsView.refs.listView.element.querySelector('.path-name').textContent).toBe("one-long-line.coffee");
140137
expect(resultsView.refs.listView.element.querySelectorAll('.preview').length).toBe(1);
141138
expect(resultsView.refs.listView.element.querySelector('.match').textContent).toBe('ghijkl');
@@ -147,7 +144,6 @@ describe('ResultsView', () => {
147144
await searchPromise;
148145

149146
resultsView = getResultsView();
150-
await resultsView.heightInvalidationPromise;
151147
expect(resultsView.refs.listView.element.querySelector('.match').textContent).toBe('ghijkl');
152148
expect(resultsView.refs.listView.element.querySelector('.match')).toHaveClass('highlight-info');
153149
expect(resultsView.refs.listView.element.querySelector('.replacement').textContent).toBe('');
@@ -179,7 +175,6 @@ describe('ResultsView', () => {
179175
await searchPromise;
180176

181177
resultsView = getResultsView();
182-
await resultsView.heightInvalidationPromise;
183178
const listElement = resultsView.refs.listView.element;
184179
expect(listElement.querySelectorAll('.match')[0].textContent).toBe('function ()');
185180
expect(listElement.querySelectorAll('.replacement')[0].textContent).toBe('() =>');
@@ -198,7 +193,6 @@ describe('ResultsView', () => {
198193
await searchPromise;
199194

200195
resultsView = getResultsView();
201-
await resultsView.heightInvalidationPromise;
202196
const {listView} = resultsView.refs;
203197
expect(listView.element.scrollTop).toBe(0);
204198
expect(listView.element.scrollHeight).toBeGreaterThan(listView.element.offsetHeight);
@@ -303,7 +297,6 @@ describe('ResultsView', () => {
303297
projectFindView.confirm();
304298
await searchPromise;
305299
resultsView = getResultsView();
306-
await resultsView.heightInvalidationPromise;
307300
});
308301

309302
it("selects the first/last item when core:move-to-top/move-to-bottom is triggered", async () => {
@@ -343,7 +336,6 @@ describe('ResultsView', () => {
343336
await searchPromise;
344337

345338
resultsView = getResultsView();
346-
await resultsView.heightInvalidationPromise;
347339

348340
resultsView.moveDown();
349341
resultsView.moveDown();
@@ -382,7 +374,6 @@ describe('ResultsView', () => {
382374
await searchPromise;
383375

384376
resultsView = getResultsView();
385-
await resultsView.heightInvalidationPromise;
386377

387378
await resultsView.collapseResult();
388379
expect(resultsView.element.querySelector('.collapsed')).not.toBe(null);
@@ -450,7 +441,6 @@ describe('ResultsView', () => {
450441
await searchPromise;
451442

452443
resultsView = getResultsView();
453-
await resultsView.heightInvalidationPromise;
454444
resultsView.selectFirstResult();
455445
});
456446

@@ -546,7 +536,6 @@ describe('ResultsView', () => {
546536
await searchPromise;
547537

548538
resultsView = getResultsView();
549-
await resultsView.heightInvalidationPromise;
550539

551540
let {length: resultCount} = resultsView.refs.listView.element.querySelectorAll(".match-row");
552541
expect(resultCount).toBe(11);
@@ -594,7 +583,6 @@ describe('ResultsView', () => {
594583
atom.commands.dispatch(projectFindView.element, 'core:confirm');
595584
await searchPromise;
596585
resultsView = getResultsView();
597-
await resultsView.heightInvalidationPromise;
598586
});
599587

600588
it("shows the preview-controls", () => {
@@ -660,11 +648,7 @@ describe('ResultsView', () => {
660648

661649
describe("when the results view is empty", () => {
662650
it("ignores core:confirm and other commands for selecting results", async () => {
663-
projectFindView.findEditor.setText('thiswillnotmatchanythingintheproject');
664-
atom.commands.dispatch(projectFindView.element, 'core:confirm');
665-
await searchPromise;
666-
resultsView = getResultsView();
667-
await resultsView.heightInvalidationPromise;
651+
const resultsView = buildResultsView({ empty: true });
668652
atom.commands.dispatch(resultsView.element, 'core:confirm');
669653
atom.commands.dispatch(resultsView.element, 'core:move-down');
670654
atom.commands.dispatch(resultsView.element, 'core:move-up');
@@ -675,10 +659,8 @@ describe('ResultsView', () => {
675659
});
676660

677661
it("won't show the preview-controls", async () => {
678-
projectFindView.findEditor.setText('thiswillnotmatchanythingintheproject');
679-
atom.commands.dispatch(projectFindView.element, 'core:confirm');
680-
await searchPromise;
681-
expect(getResultsPane().refs.previewControls.style.display).toBe('none');
662+
const resultsPane = new ResultsPaneView();
663+
expect(resultsPane.refs.previewControls.style.display).toBe('none');
682664
});
683665
});
684666

@@ -759,7 +741,6 @@ describe('ResultsView', () => {
759741
await searchPromise;
760742

761743
resultsView = getResultsView();
762-
await resultsView.heightInvalidationPromise;
763744
let fileIconClasses = Array.from(resultsView.refs.listView.element.querySelectorAll('.path-row .icon')).map(el => el.className);
764745
expect(fileIconClasses).toContain('first-icon-class second-icon-class icon');
765746
expect(fileIconClasses).toContain('third-icon-class fourth-icon-class icon');
@@ -939,7 +920,6 @@ describe('ResultsView', () => {
939920
await searchPromise;
940921

941922
resultsView = getResultsView();
942-
await resultsView.heightInvalidationPromise;
943923
});
944924

945925
it('maintains selected result when adding and removing results', async () => {

0 commit comments

Comments
 (0)