fix: View reordering on re-addition to same parent (#42085)
This commit is contained in:
parent
e2acdffe58
commit
3bd807b03e
5 changed files with 87 additions and 2 deletions
|
@ -48,6 +48,9 @@ Objects created with `new View` have the following instance methods:
|
|||
* `index` Integer (optional) - Index at which to insert the child view.
|
||||
Defaults to adding the child at the end of the child list.
|
||||
|
||||
If the same View is added to a parent which already contains it, it will be reordered such that
|
||||
it becomes the topmost view.
|
||||
|
||||
#### `view.removeChildView(view)`
|
||||
|
||||
* `view` View - Child view to remove.
|
||||
|
|
|
@ -181,6 +181,26 @@ View::~View() {
|
|||
delete view_;
|
||||
}
|
||||
|
||||
void View::ReorderChildView(gin::Handle<View> child, size_t index) {
|
||||
view_->ReorderChildView(child->view(), index);
|
||||
|
||||
const auto i = base::ranges::find(child_views_, child.ToV8());
|
||||
DCHECK(i != child_views_.end());
|
||||
|
||||
// If |view| is already at the desired position, there's nothing to do.
|
||||
const auto pos = std::next(
|
||||
child_views_.begin(),
|
||||
static_cast<ptrdiff_t>(std::min(index, child_views_.size() - 1)));
|
||||
if (i == pos)
|
||||
return;
|
||||
|
||||
if (pos < i) {
|
||||
std::rotate(pos, i, std::next(i));
|
||||
} else {
|
||||
std::rotate(i, std::next(i), std::next(pos));
|
||||
}
|
||||
}
|
||||
|
||||
void View::AddChildViewAt(gin::Handle<View> child,
|
||||
std::optional<size_t> maybe_index) {
|
||||
// TODO(nornagon): !view_ is only for supporting the weird case of
|
||||
|
@ -199,6 +219,15 @@ void View::AddChildViewAt(gin::Handle<View> child,
|
|||
|
||||
size_t index =
|
||||
std::min(child_views_.size(), maybe_index.value_or(child_views_.size()));
|
||||
|
||||
// If the child is already a child of this view, just reorder it.
|
||||
// This matches the behavior of View::AddChildViewAtImpl and
|
||||
// otherwise will CHECK if the same view is added multiple times.
|
||||
if (child->view()->parent() == view_) {
|
||||
ReorderChildView(child, index);
|
||||
return;
|
||||
}
|
||||
|
||||
child_views_.emplace(child_views_.begin() + index, // index
|
||||
isolate(), child->GetWrapper()); // v8::Global(args...)
|
||||
#if BUILDFLAG(IS_MAC)
|
||||
|
@ -221,7 +250,7 @@ void View::RemoveChildView(gin::Handle<View> child) {
|
|||
return;
|
||||
if (!child->view())
|
||||
return;
|
||||
auto it = std::find(child_views_.begin(), child_views_.end(), child.ToV8());
|
||||
const auto it = base::ranges::find(child_views_, child.ToV8());
|
||||
if (it != child_views_.end()) {
|
||||
#if BUILDFLAG(IS_MAC)
|
||||
ScopedCAActionDisabler disable_animations;
|
||||
|
|
|
@ -57,6 +57,8 @@ class View : public gin_helper::EventEmitter<View>, public views::ViewObserver {
|
|||
void set_delete_view(bool should) { delete_view_ = should; }
|
||||
|
||||
private:
|
||||
void ReorderChildView(gin::Handle<View> child, size_t index);
|
||||
|
||||
std::vector<v8::Global<v8::Object>> child_views_;
|
||||
|
||||
bool delete_view_ = true;
|
||||
|
|
|
@ -22,4 +22,36 @@ describe('View', () => {
|
|||
w.contentView.addChildView(w.contentView);
|
||||
}).to.throw('A view cannot be added as its own child');
|
||||
});
|
||||
|
||||
it('does not crash when attempting to add a child multiple times', () => {
|
||||
w = new BaseWindow({ show: false });
|
||||
const cv = new View();
|
||||
w.setContentView(cv);
|
||||
|
||||
const v = new View();
|
||||
w.contentView.addChildView(v);
|
||||
w.contentView.addChildView(v);
|
||||
w.contentView.addChildView(v);
|
||||
|
||||
expect(w.contentView.children).to.have.lengthOf(1);
|
||||
});
|
||||
|
||||
it('correctly reorders children', () => {
|
||||
w = new BaseWindow({ show: false });
|
||||
const cv = new View();
|
||||
w.setContentView(cv);
|
||||
|
||||
const v1 = new View();
|
||||
const v2 = new View();
|
||||
const v3 = new View();
|
||||
w.contentView.addChildView(v1);
|
||||
w.contentView.addChildView(v2);
|
||||
w.contentView.addChildView(v3);
|
||||
|
||||
expect(w.contentView.children).to.deep.equal([v1, v2, v3]);
|
||||
|
||||
w.contentView.addChildView(v1);
|
||||
w.contentView.addChildView(v2);
|
||||
expect(w.contentView.children).to.deep.equal([v3, v1, v2]);
|
||||
});
|
||||
});
|
||||
|
|
|
@ -36,6 +36,25 @@ describe('WebContentsView', () => {
|
|||
v.removeChildView(wcv);
|
||||
});
|
||||
|
||||
it('correctly reorders children', () => {
|
||||
const w = new BaseWindow({ show: false });
|
||||
const cv = new View();
|
||||
w.setContentView(cv);
|
||||
|
||||
const wcv1 = new WebContentsView();
|
||||
const wcv2 = new WebContentsView();
|
||||
const wcv3 = new WebContentsView();
|
||||
w.contentView.addChildView(wcv1);
|
||||
w.contentView.addChildView(wcv2);
|
||||
w.contentView.addChildView(wcv3);
|
||||
|
||||
expect(w.contentView.children).to.deep.equal([wcv1, wcv2, wcv3]);
|
||||
|
||||
w.contentView.addChildView(wcv1);
|
||||
w.contentView.addChildView(wcv2);
|
||||
expect(w.contentView.children).to.deep.equal([wcv3, wcv1, wcv2]);
|
||||
});
|
||||
|
||||
function triggerGCByAllocation () {
|
||||
const arr = [];
|
||||
for (let i = 0; i < 1000000; i++) {
|
||||
|
@ -79,7 +98,7 @@ describe('WebContentsView', () => {
|
|||
expect(await v.webContents.executeJavaScript('initialVisibility')).to.equal('hidden');
|
||||
});
|
||||
|
||||
it('becomes visibile when attached', async () => {
|
||||
it('becomes visible when attached', async () => {
|
||||
const v = new WebContentsView();
|
||||
await v.webContents.loadURL('about:blank');
|
||||
expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('hidden');
|
||||
|
|
Loading…
Reference in a new issue