Remove Ctrl-K shortcut indicator from modal #1945
Conversation
The shortcut is already present on the navbar button that will show the modal, thus it is likely unnecessary on the modal itself. As pointed out in pydata#1944, this is also super crowded on mobile, so less element is actually cleaner. This also remove Ctrl-K from the search field on the search page, but again, It is likely unnecessary there, as the user is already on the search page. In general I think removing is better than having it as an option as it is less maintenance. Closes pydata#1944
Coverage report
This PR does not seem to contain any modification to coverable code. |
|
Thanks for picking this up! Note that there are some projects, that only have a search icon in the nav bar, e.g. numpy or matplotlib. At least for matplotlib I can say that it's a matter of available space and switching to a search field (in particular a long one including the shortcuts) is not easily possible. I therefore very much favor keeping the shortcut in the modal dialog on larger screens. Instead of deleting the html you could set its CSS to |
|
That's a good point. I'm wondering how easy it would be to achieve with custom css instead of an option in PST. IF they customise the search button in the nav bar, then maybe we can ask them to also modify extend the search field. I'll look into it. |
Closes pydata#1944. Alternative to pydata#1945.
|
@Carreau I suspect "extend the search field" is difficult as you would have to inject HTML. OTOH my proposal to hide the shortcut on small screens is fairly simple with pure CSS. I've opened #1947 to demonstrate this. I think on large screens it's ok or even desirable to still show the shortcut in the search field popup. |
Closes #1944. Alternative to #1945. While #1945 removes the shortcut completely, I've [suggested that it actually has value](#1945 (comment)) and should only be removed on smaller screens. That's what this PR implements. It shows the shortcuts if the window size is at least "md" i.e. >=768px (I figured, this is a reasonable threshold - see discussion below, but if you think other values are better, that's perfectly fine). See pull requests for screenshots.
The shortcut is already present on the navbar button that will show the modal, thus it is likely unnecessary on the modal itself.
As pointed out in #1944, this is also super crowded on mobile, so less element is actually cleaner.
This also remove Ctrl-K from the search field on the search page, but again, It is likely unnecessary there, as the user is already on the search page.
In general I think removing is better than having it as an option as it is less maintenance.
Closes #1944