Add note about input prompt scaling, and add options to change the key to start and restart searches#10
Open
microraptor wants to merge 2 commits intoCogentRedTester:masterfrom
Open
Add note about input prompt scaling, and add options to change the key to start and restart searches#10microraptor wants to merge 2 commits intoCogentRedTester:masterfrom
microraptor wants to merge 2 commits intoCogentRedTester:masterfrom
Conversation
This setting seems very important for high DPI displays
These change the default keybindings to open a search, as well as to restart a search, while a page is open
CogentRedTester
requested changes
Jan 17, 2023
|
|
||
| function list_meta:open_wrapper(advanced) | ||
| if self.keyword == nil then self.empty_text = "Press f12 to enter search query" end | ||
| if self.keyword == nil then self.empty_text = "Enter a new search query" end |
Owner
There was a problem hiding this comment.
I'm fine with this change.
Comment on lines
+197
to
+198
| {o.keybind_open_search, "open_search", function() temp:get_input(false) end, {}}, | ||
| {o.keybind_open_flag_search, "open_flag_search", function() temp:get_input(true) end, {}} |
Owner
There was a problem hiding this comment.
Most of this PR is actually unnecessary. If we remove these two lines:
Suggested change
| {o.keybind_open_search, "open_search", function() temp:get_input(false) end, {}}, | |
| {o.keybind_open_flag_search, "open_flag_search", function() temp:get_input(true) end, {}} |
And change the open_wrapper function to be:
function list_meta:open_wrapper(advanced)
if self.keyword == nil then self.empty_text = "Press f12 to enter search query" end
local hidden = self.hidden
if hidden then self:open() end
if self.keyword == nil or not hidden then self:get_input(advanced) end
endThen the standard mpv keybind to open the page will be also be used when the page is open instead of being overridden. Then the keybinds can be changed by using standard input.conf changes.
Comment on lines
+114
to
+119
| The scale of the input prompt can be changed with this setting in the `mpv.conf`: | ||
|
|
||
| ```txt | ||
| script-opts-append=user_input-scale=1 | ||
| ``` | ||
|
|
Owner
There was a problem hiding this comment.
Instead of recommending an entry in mpv.conf I would instead prefer to recommend an entry in ~~/script-opts/user_input.conf which would contain scale=1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR has two commits:
The first one adds only a small note to the readme how the input prompt text scaling can be changed. This setting seems very important for high DPI displays. I have a UHD monitor and scale=1 is not readable and scale=7 comfortable.
I have the keybind to open the script on Ctrl+f instead of f12, because it is more intuitive and better for laptop keyboards and fancy keyboards, which have the F-keys hidden behind a second layer. Changing the keybinding only in
input.confleads to the issue, that restarting a search when a page is already opened is un-intuitive. So the second commit changes only a few lines of the code to allow options to change the f12 default.I also changed the text
Press f12 to enter search querytoEnter a new search query, because this text is usually shown after the keybind has been pressed and is therefore misleading.