diff --git a/buildutils/src/rollup.tests.config.js b/buildutils/src/rollup.tests.config.js index d128aee17..556c998a0 100644 --- a/buildutils/src/rollup.tests.config.js +++ b/buildutils/src/rollup.tests.config.js @@ -5,13 +5,15 @@ import nodeResolve from '@rollup/plugin-node-resolve'; import commonjs from '@rollup/plugin-commonjs'; +import sourcemaps from 'rollup-plugin-sourcemaps'; export function createRollupTestConfig(options) { return { input: './lib/index.spec.js', output: { - file: './lib/bundle.test.js' + file: './lib/bundle.test.js', + sourcemap: true }, - plugins: [commonjs(), nodeResolve()] + plugins: [commonjs(), nodeResolve(), sourcemaps()] }; } diff --git a/packages/widgets/src/commandpalette.ts b/packages/widgets/src/commandpalette.ts index 087d05d40..5559e5076 100644 --- a/packages/widgets/src/commandpalette.ts +++ b/packages/widgets/src/commandpalette.ts @@ -295,7 +295,9 @@ export class CommandPalette extends Widget { * A message handler invoked on an `'update-request'` message. */ protected onUpdateRequest(msg: Message): void { - if (this.isHidden) { + if (!this.isVisible) { + // Ensure to clear the content if the widget is hidden + VirtualDOM.render(null, this.contentNode); return; } diff --git a/packages/widgets/src/widget.ts b/packages/widgets/src/widget.ts index 53bf771fe..7cfec3a7c 100644 --- a/packages/widgets/src/widget.ts +++ b/packages/widgets/src/widget.ts @@ -112,6 +112,10 @@ export class Widget implements IMessageHandler, IObservableDisposable { /** * Test whether the widget is explicitly hidden. + * + * #### Notes + * You should prefer `!{@link isVisible}` over `{@link isHidden}` if you want to know if the + * widget is hidden as this does not test if the widget is hidden because one of its ancestors is hidden. */ get isHidden(): boolean { return this.testFlag(Widget.Flag.IsHidden); @@ -123,9 +127,20 @@ export class Widget implements IMessageHandler, IObservableDisposable { * #### Notes * A widget is visible when it is attached to the DOM, is not * explicitly hidden, and has no explicitly hidden ancestors. + * + * Since 2.7.0, this does not rely on the {@link Widget.Flag.IsVisible} flag. + * It recursively checks the visibility of all parent widgets. */ get isVisible(): boolean { - return this.testFlag(Widget.Flag.IsVisible); + // eslint-disable-next-line @typescript-eslint/no-this-alias + let parent: Widget | null = this; + do { + if (parent.isHidden || !parent.isAttached) { + return false; + } + parent = parent.parent; + } while (parent != null); + return true; } /** @@ -482,6 +497,9 @@ export class Widget implements IMessageHandler, IObservableDisposable { * * #### Notes * This will not typically be called directly by user code. + * + * Since 2.7.0, {@link Widget.Flag.IsVisible} is deprecated. + * It will be removed in a future version. */ testFlag(flag: Widget.Flag): boolean { return (this._flags & flag) !== 0; @@ -492,6 +510,9 @@ export class Widget implements IMessageHandler, IObservableDisposable { * * #### Notes * This will not typically be called directly by user code. + * + * Since 2.7.0, {@link Widget.Flag.IsVisible} is deprecated. + * It will be removed in a future version. */ setFlag(flag: Widget.Flag): void { this._flags |= flag; @@ -502,6 +523,9 @@ export class Widget implements IMessageHandler, IObservableDisposable { * * #### Notes * This will not typically be called directly by user code. + * + * Since 2.7.0, {@link Widget.Flag.IsVisible} is deprecated. + * It will be removed in a future version. */ clearFlag(flag: Widget.Flag): void { this._flags &= ~flag; @@ -856,6 +880,9 @@ export namespace Widget { /** * The widget is visible. + * + * @deprecated since 2.7.0, apply that flag consistently was not reliable + * so it was dropped in favor of a recursive check of the visibility of all parents. */ IsVisible = 0x8, diff --git a/packages/widgets/tests/src/commandpalette.spec.ts b/packages/widgets/tests/src/commandpalette.spec.ts index 53f7c34ff..ce758c625 100644 --- a/packages/widgets/tests/src/commandpalette.spec.ts +++ b/packages/widgets/tests/src/commandpalette.spec.ts @@ -44,6 +44,7 @@ describe('@lumino/widgets', () => { beforeEach(() => { commands = new CommandRegistry(); palette = new CommandPalette({ commands }); + Widget.attach(palette, document.body); }); afterEach(() => { @@ -493,65 +494,119 @@ describe('@lumino/widgets', () => { }); let palette = new CommandPalette({ commands }); - palette.addItem({ command: 'example:cut', category: 'Edit' }); - palette.addItem({ command: 'example:copy', category: 'Edit' }); - palette.addItem({ command: 'example:paste', category: 'Edit' }); - palette.addItem({ command: 'example:one', category: 'Number' }); - palette.addItem({ command: 'example:two', category: 'Number' }); - palette.addItem({ command: 'example:three', category: 'Number' }); - palette.addItem({ command: 'example:four', category: 'Number' }); - palette.addItem({ command: 'example:black', category: 'Number' }); - palette.addItem({ command: 'example:new-tab', category: 'File' }); - palette.addItem({ command: 'example:close-tab', category: 'File' }); - palette.addItem({ command: 'example:save-on-exit', category: 'File' }); - palette.addItem({ - command: 'example:open-task-manager', - category: 'File' - }); - palette.addItem({ command: 'example:close', category: 'File' }); - palette.addItem({ - command: 'example:clear-cell', - category: 'Notebook Cell Operations' - }); - palette.addItem({ - command: 'example:cut-cells', - category: 'Notebook Cell Operations' - }); - palette.addItem({ - command: 'example:run-cell', - category: 'Notebook Cell Operations' - }); - palette.addItem({ command: 'example:cell-test', category: 'Console' }); - palette.addItem({ command: 'notebook:new', category: 'Notebook' }); - palette.id = 'palette'; - - // Search the command palette: update the inputNode, then force a refresh - palette.inputNode.value = 'clea'; + Widget.attach(palette, document.body); + try { + palette.addItem({ command: 'example:cut', category: 'Edit' }); + palette.addItem({ command: 'example:copy', category: 'Edit' }); + palette.addItem({ command: 'example:paste', category: 'Edit' }); + palette.addItem({ command: 'example:one', category: 'Number' }); + palette.addItem({ command: 'example:two', category: 'Number' }); + palette.addItem({ command: 'example:three', category: 'Number' }); + palette.addItem({ command: 'example:four', category: 'Number' }); + palette.addItem({ command: 'example:black', category: 'Number' }); + palette.addItem({ command: 'example:new-tab', category: 'File' }); + palette.addItem({ command: 'example:close-tab', category: 'File' }); + palette.addItem({ + command: 'example:save-on-exit', + category: 'File' + }); + palette.addItem({ + command: 'example:open-task-manager', + category: 'File' + }); + palette.addItem({ command: 'example:close', category: 'File' }); + palette.addItem({ + command: 'example:clear-cell', + category: 'Notebook Cell Operations' + }); + palette.addItem({ + command: 'example:cut-cells', + category: 'Notebook Cell Operations' + }); + palette.addItem({ + command: 'example:run-cell', + category: 'Notebook Cell Operations' + }); + palette.addItem({ + command: 'example:cell-test', + category: 'Console' + }); + palette.addItem({ command: 'notebook:new', category: 'Notebook' }); + palette.id = 'palette'; + + // Search the command palette: update the inputNode, then force a refresh + palette.inputNode.value = 'clea'; + palette.refresh(); + MessageLoop.flush(); + + // Expect that headers and items appear in descending score order, + // even if the same header occurs multiple times. + const children = palette.contentNode.children; + expect(children.length).to.equal(7); + expect(children[0].textContent).to.equal('Notebook Cell Operations'); + expect(children[1].getAttribute('data-command')).to.equal( + 'example:clear-cell' + ); + // The next match should be from a different category + expect(children[2].textContent).to.equal('File'); + expect(children[3].getAttribute('data-command')).to.equal( + 'example:close-tab' + ); + // The next match should be the same as in a previous category + expect(children[4].textContent).to.equal('Notebook Cell Operations'); + expect(children[5].getAttribute('data-command')).to.equal( + 'example:cut-cells' + ); + // The next match has the same category as the previous one did, so the header is not repeated + expect(children[6].getAttribute('data-command')).to.equal( + 'example:run-cell' + ); + } finally { + palette.dispose(); + } + }); + + it('should clear the widget content if hidden', () => { + commands.addCommand('test', { execute: () => {}, label: 'test' }); + palette.addItem({ command: 'test', category: 'test' }); + const content = palette.contentNode; + const itemClass = '.lm-CommandPalette-item'; + const items = () => content.querySelectorAll(itemClass); + palette.refresh(); MessageLoop.flush(); - // Expect that headers and items appear in descending score order, - // even if the same header occurs multiple times. - const children = palette.contentNode.children; - expect(children.length).to.equal(7); - expect(children[0].textContent).to.equal('Notebook Cell Operations'); - expect(children[1].getAttribute('data-command')).to.equal( - 'example:clear-cell' - ); - // The next match should be from a different category - expect(children[2].textContent).to.equal('File'); - expect(children[3].getAttribute('data-command')).to.equal( - 'example:close-tab' - ); - // The next match should be the same as in a previous category - expect(children[4].textContent).to.equal('Notebook Cell Operations'); - expect(children[5].getAttribute('data-command')).to.equal( - 'example:cut-cells' - ); - // The next match has the same category as the previous one did, so the header is not repeated - expect(children[6].getAttribute('data-command')).to.equal( - 'example:run-cell' - ); + expect(items()).to.have.length(1); + + palette.hide(); + palette.refresh(); + MessageLoop.flush(); + expect(items()).to.have.length(0); + }); + + it('should clear the widget content if container is hidden', () => { + const parent = new Widget(); + Widget.attach(parent, document.body); + try { + palette.parent = parent; + commands.addCommand('test', { execute: () => {}, label: 'test' }); + palette.addItem({ command: 'test', category: 'test' }); + const content = palette.contentNode; + const itemClass = '.lm-CommandPalette-item'; + const items = () => content.querySelectorAll(itemClass); + + palette.refresh(); + MessageLoop.flush(); + + expect(items()).to.have.length(1); + + parent.hide(); + palette.refresh(); + MessageLoop.flush(); + expect(items()).to.have.length(0); + } finally { + parent.dispose(); + } }); }); @@ -559,11 +614,14 @@ describe('@lumino/widgets', () => { it('should handle click, keydown, and input events', () => { let palette = new LogPalette({ commands }); Widget.attach(palette, document.body); - ['click', 'keydown', 'input'].forEach(type => { - palette.node.dispatchEvent(new Event(type, { bubbles })); - expect(palette.events).to.contain(type); - }); - palette.dispose(); + try { + ['click', 'keydown', 'input'].forEach(type => { + palette.node.dispatchEvent(new Event(type, { bubbles })); + expect(palette.events).to.contain(type); + }); + } finally { + palette.dispose(); + } }); context('click', () => { @@ -572,7 +630,6 @@ describe('@lumino/widgets', () => { commands.addCommand('test', { execute: () => (called = true) }); palette.addItem(defaultOptions); - Widget.attach(palette, document.body); MessageLoop.flush(); let node = palette.contentNode.querySelector( @@ -587,7 +644,6 @@ describe('@lumino/widgets', () => { commands.addCommand('test', { execute: () => (called = true) }); palette.addItem(defaultOptions); - Widget.attach(palette, document.body); MessageLoop.flush(); let node = palette.contentNode.querySelector( @@ -604,7 +660,6 @@ describe('@lumino/widgets', () => { let content = palette.contentNode; palette.addItem(defaultOptions); - Widget.attach(palette, document.body); MessageLoop.flush(); let node = content.querySelector('.lm-mod-active'); @@ -625,7 +680,6 @@ describe('@lumino/widgets', () => { let content = palette.contentNode; palette.addItem(defaultOptions); - Widget.attach(palette, document.body); MessageLoop.flush(); let node = content.querySelector('.lm-mod-active'); @@ -647,7 +701,6 @@ describe('@lumino/widgets', () => { let content = palette.contentNode; palette.addItem(defaultOptions); - Widget.attach(palette, document.body); MessageLoop.flush(); let node = content.querySelector('.lm-mod-active'); @@ -677,7 +730,6 @@ describe('@lumino/widgets', () => { let content = palette.contentNode; palette.addItem(defaultOptions); - Widget.attach(palette, document.body); MessageLoop.flush(); expect(content.querySelector('.lm-mod-active')).to.equal(null); @@ -704,7 +756,6 @@ describe('@lumino/widgets', () => { palette.addItem({ command: name, category: 'test' }); }); - Widget.attach(palette, document.body); MessageLoop.flush(); let content = palette.contentNode; @@ -734,7 +785,6 @@ describe('@lumino/widgets', () => { }); }); - Widget.attach(palette, document.body); MessageLoop.flush(); let headers = () => diff --git a/packages/widgets/tests/src/tabbar.spec.ts b/packages/widgets/tests/src/tabbar.spec.ts index 0479aad45..f563c8fd8 100644 --- a/packages/widgets/tests/src/tabbar.spec.ts +++ b/packages/widgets/tests/src/tabbar.spec.ts @@ -1815,6 +1815,7 @@ describe('@lumino/widgets', () => { * TODO: * Find a way to trigger the change of focus. */ + /* it.skip('should keep focus on the second tab on tabulation', () => { const node = document.createElement('div'); node.setAttribute('tabindex', '0'); @@ -1844,6 +1845,7 @@ describe('@lumino/widgets', () => { ); expect(document.activeElement).to.equal(secondTab); }); + */ }); context('contextmenu', () => { diff --git a/packages/widgets/tests/src/widget.spec.ts b/packages/widgets/tests/src/widget.spec.ts index 765352a4a..2aa082b11 100644 --- a/packages/widgets/tests/src/widget.spec.ts +++ b/packages/widgets/tests/src/widget.spec.ts @@ -253,6 +253,15 @@ describe('@lumino/widgets', () => { let widget = new Widget(); expect(widget.isVisible).to.equal(false); }); + + it('should be false if the widget is not hidden but its parent is hidden', () => { + let parent = new Widget(); + parent.hide(); + let widget = new Widget(); + widget.parent = parent; + expect(widget.isHidden).to.equal(false); + expect(widget.isVisible).to.equal(false); + }); }); describe('#node', () => { diff --git a/review/api/widgets.api.md b/review/api/widgets.api.md index d38bd1d67..5d023b1c0 100644 --- a/review/api/widgets.api.md +++ b/review/api/widgets.api.md @@ -1364,6 +1364,7 @@ export namespace Widget { IsAttached = 2, IsDisposed = 1, IsHidden = 4, + // @deprecated IsVisible = 8 } export enum HiddenMode {