-
Notifications
You must be signed in to change notification settings - Fork 350
Add Quick-Switch modal #1028
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Quick-Switch modal #1028
Conversation
This looks great @cybrox! ❤️ A couple things:
I would also add the "search" icon at the beginning of the search input. But I am not a designer. :) @archdragon, do you have any thoughts here?
|
Oh, one more:
|
I agree, a lighter design (in terms of less boder-y) looks better. Also light background would probably look very good, I will definitely change that. As for the full-width suggestion, I like it in slack but I don't think there's a real benefit here. The package names are very short, so it'd be lots of empty space, I guess.
It's set in the
Will add that!
Whoops, I completely forgot about that. I will definitely add that as well. |
The package names are very short,
Try searching for „nerves” :D
… Wiadomość napisana przez Sven Gehring ***@***.***> w dniu 22.06.2019, o godz. 11:48:
@cybrox commented on this pull request.
In assets/js/quick-switch.js:
> + prevResult.addClass('selected')
+ autoCompleteSelected -= 1
+ } else {
+ selectLastAcResult()
+ }
+}
+
+function deselectAcResult () {
+ $('.quick-switch-result.selected').removeClass('selected')
+ autoCompleteSelected = -1
+}
+
+// Public Methods
+// --------------
+
+export { openQuickSwichModal }
✏️ typo
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Changes
I have updated the Demo as well. Edit: I have no clue why the test for the dist file naming is failing 🤔 |
Excellent! Three more notes:
I am not sure if this issue also exists in #1031.
Note I also replaced "another" by "a". That's because if we are going to make this available locally, then this will show up even if you don't have a package, so "another" could be misleading. We would have to change this on other places too. |
You probably have multiple .dist files around because of a rebase or changes in master. To fix it, do another full rebase against master, call |
|
For 2. I noticed that when you hover an item and leave your mouse in place, the autocomplete suggestions will update "over" the mouse (you don't see it) but the item where your mouse is located will automatically be selected. This happens on the sidebar search autocomplete as well. Could it be that this is what happened? |
@cybrox sounds like it. But FWIW I also don't mind the current behaviour of the sidebar and I would be OK with keeping both simple. |
For the sake of keeping things simple, I have closed #1031 and adjusted the code here to behave the same as the sidebar autocomplete. (FYI: I'm not trying to re-use the sidebar autocomplete code, because if we add support for going to a specific version directly, the code will differ in quite a few places) With this, all discussed issues should be solved 🎉 |
❤️ 💚 💙 💛 💜 |
This is the implementation for #1008. I have not yet added the version select functionality discussed in #1026, but want to do so once we have discussed possible changes required for this PR.
This PR adds a 'quick-switch' modal that is opened by pressing g and can be closed by pressing Esc, similar to the keyboard shortcut help modal. You can enter any term and hit enter, and it will directly take you to the corresponding docs on
hexdocs.pm
. Alternatively, if you stop typing for 300ms* it will load matching suggestions from the API and display an auto-complete list with the most accurate 10* results. The behaviour of the auto-complete list is pretty much the same as in the sidebar.* These values felt good but are up for debate.
For testing, check out the Live Demo. (That is inconveniently hosted on a server that was just moved, if it doesn't load, try flushing your dns cache, sorry)