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.
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
[WIP] Implemented
keybind
module to handle keyboard shortcuts #186[WIP] Implemented
keybind
module to handle keyboard shortcuts #186Changes from 1 commit
e57f090
25c8a19
dae5d01
e83a78a
1621c54
2b0f882
ccc5abd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
When
Key
is a type, this can be written asThere 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.
is a bit more descriptive. The type is also pretty self-explanatory and the TsDoc string therefore only clutters it a bit and could be removed
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.
should take
Refs
as a parameter and use this instead ofgetElementsByClassName
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.
should take
Refs
as a parameter (withSearchBar
added to the type) and use this instead ofgetElemtById
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.
could be written as
with related
.open()
and.hasFocus()
functions inResult.svelte
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.
could be written as
with related
.openResultInNewTab()
and.hasFocus()
functions inResult.svelte
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.
should not directly reference the dom elements but instead call related
.userQuery()
andsearch(`${query} site:${resultLink.hostname}`)
methods.ResultLink.svelte
should also have a.url()
methodThere 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.
can you expand on
.userQuery()
&search(...)
? I couldn't find anyuserQuery
methods anywhere and was just going to pull it fromPageData
and pass it in a wrapper. As forsearch
I was going to use this if that's what you were referring to. Wasn't sure if these are existing need to be built out.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.
Yea I think it would be easiest to create those as new methods in
Searchbar.svelte
with an added
formElem
bindThe search function in
search.ts
effectively just calls the api and returns the results. Calling the search function inSearchbar.svelte
would ensure that the user interface gets updated correctly with the new results.domainSearch
can then be something likeWhere
getMainLink
would be a new method inResult.svelte
and
url
would be a new method inResultLink.svelte
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.
Should use
Refs
with a binding for the mispell link added to the type instead of.getElementById
and could then also use a.open()
methodThere 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.
this could be