Search: define which element & event shows the modal #58
Conversation
Define two new attributes `show-modal-selector` and `show-modal-event` to allow the user defining which HTML element will show the modal. In this example, when clicking on the `input` it will show the modal.
|
This is another example towards the definition of the pattern we want to implement. It answers the question what's the selector for the search-as-you-type be triggered? from #13 I'm not 100 sure about this pattern yet, but I opened this PR as an example and to start receiving feedback. However, I think it's simple enough and easy to customize and maintain, I think. |
| @@ -490,13 +494,28 @@ export class SearchElement extends LitElement { | |||
| } | |||
| }; | |||
|
|
|||
| _handleShowModalUser = (e) => { | |||
There was a problem hiding this comment.
Any reason this isn't a normal class method instead of an anonymous function? It seems to be used as a method, given this.showModal()
There was a problem hiding this comment.
Yes. There is a reason for this,
There was a problem hiding this comment.
Gotcha, that makes sense. I was going to suggest a static method, but I think in that scope this refers to the class itself.
Format the code before pushing.
| @@ -64,7 +66,9 @@ export class SearchElement extends LitElement { | |||
| this.currentQueryRequest = null; | |||
| this.defaultFilter = null; | |||
| this.filters = []; | |||
| this.showModalKeycode = 191; | |||
| this.triggerKeycode = 191; | |||
There was a problem hiding this comment.
@agjohnson should we have a trigger-modifier here or similar? See #57
How can we make people to use Ctrl + K for example to show the search?
There was a problem hiding this comment.
Good question. I haven't done this, but it looks like we can get the modifier state from the keydown event passed into our event handler function.
I will say that we should be careful with defaults here though. It's easy to override native key bindings when working with modifier key presses.
There was a problem hiding this comment.
Is it fine to add a trigger-modifier-keycode attribute for this? 🤔
There was a problem hiding this comment.
Yeah, something like that, though I don't think keycode is the most correct. I believe this is the KeyboardEvent method you'd want to use:
https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/getModifierState
So, more like <readthedocs-search trigger-key-modifier="control"> or something.
|
I think this is ready to merge. I asked a question, but it can probably be achieved in a different PR. Let me know. |
Define two new attributes
show-modal-selectorandshow-modal-eventto allow the user defining which HTML element will show the modal.In this example, when clicking on the
inputit will show the modal.