-
Notifications
You must be signed in to change notification settings - Fork 6k
Ability to disable browser context menu #38682
Changes from 16 commits
b543526
8d38b6b
733ba64
3503fd4
1bce49f
98cdb32
f25fbca
4fd56bd
acbc3be
4ace58d
4bc3f04
a2c5a88
a0e294b
ad8bc4e
f626594
edb57c3
781148d
85b6d88
167babb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -48,6 +48,12 @@ class FullPageEmbeddingStrategy extends EmbeddingStrategy { | |||||
registerElementForCleanup(resourceHost); | ||||||
} | ||||||
|
||||||
@override | ||||||
void disableContextMenu() => disableContextMenuOn(domDocument.body!); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We usually set these event listeners on
Suggested change
|
||||||
|
||||||
@override | ||||||
void enableContextMenu() => enableContextMenuOn(domDocument.body!); | ||||||
|
||||||
void _setHostAttribute(String name, String value) { | ||||||
domDocument.body!.setAttribute(name, value); | ||||||
} | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -121,4 +121,54 @@ void doTests() { | |
reason: 'Should be injected `nextTo` the passed element.'); | ||
}); | ||
}); | ||
|
||
group('context menu', () { | ||
setUp(() { | ||
target = createDomElement('this-is-the-target'); | ||
domDocument.body!.append(target); | ||
strategy = CustomElementEmbeddingStrategy(target); | ||
strategy.initialize(); | ||
}); | ||
|
||
tearDown(() { | ||
target.remove(); | ||
}); | ||
|
||
test('disableContextMenu and enableContextMenu can toggle the context menu', () { | ||
// When the app starts, contextmenu events are not prevented. | ||
DomEvent event = createDomEvent('Event', 'contextmenu'); | ||
expect(event.defaultPrevented, isFalse); | ||
target.dispatchEvent(event); | ||
expect(event.defaultPrevented, isFalse); | ||
|
||
// Disabling the context menu causes contextmenu events to be prevented. | ||
strategy.disableContextMenu(); | ||
event = createDomEvent('Event', 'contextmenu'); | ||
expect(event.defaultPrevented, isFalse); | ||
target.dispatchEvent(event); | ||
expect(event.defaultPrevented, isTrue); | ||
Comment on lines
+144
to
+149
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another important test is to dispatch an event outside of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great idea, thanks! |
||
|
||
// Disabling again has no effect. | ||
strategy.disableContextMenu(); | ||
event = createDomEvent('Event', 'contextmenu'); | ||
expect(event.defaultPrevented, isFalse); | ||
target.dispatchEvent(event); | ||
expect(event.defaultPrevented, isTrue); | ||
|
||
// Enabling the context menu means that contextmenu events are back to not | ||
// being prevented. | ||
strategy.enableContextMenu(); | ||
event = createDomEvent('Event', 'contextmenu'); | ||
expect(event.defaultPrevented, isFalse); | ||
target.dispatchEvent(event); | ||
expect(event.defaultPrevented, isFalse); | ||
|
||
// Enabling again has no effect. | ||
strategy.enableContextMenu(); | ||
event = createDomEvent('Event', 'contextmenu'); | ||
expect(event.defaultPrevented, isFalse); | ||
target.dispatchEvent(event); | ||
expect(event.defaultPrevented, isFalse); | ||
}); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've omitted any web or browser namespacing (not "flutter/webcontextmenu") because other methods don't seem to include platform names like that. And maybe in the future this could be used on other platforms.
In the framework PR I'll make the API specific (BrowserContextMenu.disable or something like that).