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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small usability improvement for users with JavaScript disabled (low priority) #649

Merged
merged 1 commit into from Oct 22, 2017

Conversation

3 participants
@ericcornelissen
Copy link
Contributor

ericcornelissen commented Oct 17, 2017

By default the search input is now disabled and when hovered displays a message informing the user search is only available when JavaScript is enabled. When JavaScript is enabled, the input is programmatically enabled and the title is changed as well.

Feedback is welcome 馃檪

@davidklebanoff

This comment has been minimized.

Copy link
Member

davidklebanoff commented Oct 17, 2017

Sounds good to me, but if/when you implement #648, it seems to me the fallback solution would be to have a search button that submits the search form with an HTTP get with the query parameter, reloading the page. So this PR would be effectively removed when that's complete.

@ericcornelissen

This comment has been minimized.

Copy link
Contributor Author

ericcornelissen commented Oct 17, 2017

You bring up an interesting point 馃槄 But I will have to look into how that will be implemented as I'm not sure if HTTP get request work with Jekyll [ref], if not that feature will/can be implemented using JavaScript...

Edit: As far as I could find it is not possible to implement the search form using HTTP get requests in Jekyll, so it would not render this PR obsolete

@birjolaxew birjolaxew merged commit 5dd166d into simple-icons:develop Oct 22, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@birjolaxew

This comment has been minimized.

Copy link
Member

birjolaxew commented Oct 22, 2017

馃憤 Great idea! An alternative solution would've been to use <noscript>, but this seems to be fine as-is. Merged!

@ericcornelissen ericcornelissen deleted the ericcornelissen:usability-noscript branch Nov 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment