Skip to content
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

Fix search input #791

Merged
merged 4 commits into from
Dec 18, 2015
Merged

Fix search input #791

merged 4 commits into from
Dec 18, 2015

Conversation

kwm4385
Copy link
Contributor

@kwm4385 kwm4385 commented Dec 8, 2015

After rendering move the cursor to the end of the input and keep it focused if there is text.

@wsorenson

@wsorenson
Copy link
Contributor

Does this prevent it from occasionally erasing what was in there?

@kwm4385
Copy link
Contributor Author

kwm4385 commented Dec 8, 2015

While I wasn't able to repro that myself, I think it's due to it being overwritten as you type and the refresh happens. If that still happens after this is applied let me know.

@@ -49,4 +49,10 @@ class View extends Backbone.View
next: '<span class="glyphicon glyphicon-chevron-right"></span>'
$('table.paginated').css('display', 'table');

searchInput = $('.big-search-box')
strLength = searchInput.val().length * 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why * 2 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to make sure it always ends up at the end, there's some cases where length would fail us (Opera sometimes sees a carriage return as 2 characters)

@tpetr
Copy link
Contributor

tpetr commented Dec 10, 2015

This could still have issues if the cursor is in the middle of the text -- maybe we can just get the Search box to not re-render?

@@ -49,4 +49,10 @@ class View extends Backbone.View
next: '<span class="glyphicon glyphicon-chevron-right"></span>'
$('table.paginated').css('display', 'table');

searchInput = $('.big-search-box')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small, but i would rename to $searchInput for clarity

@kwm4385
Copy link
Contributor Author

kwm4385 commented Dec 11, 2015

@tpetr Looking at the code it seems it won't be that easy. The table is a subview of the search, making it the top level element. If we don't re-render that, nothing else can. :/

@kwm4385
Copy link
Contributor Author

kwm4385 commented Dec 15, 2015

I did some more research and found a better solution for this. Now it'll save your caret position before rendering and reset it afterwards. This will also preserve any selections as well.

@tpetr
Copy link
Contributor

tpetr commented Dec 18, 2015

💥 thanks

@tpetr tpetr added this to the 0.4.6 milestone Dec 18, 2015
tpetr pushed a commit that referenced this pull request Dec 18, 2015
@tpetr tpetr merged commit c650f7d into master Dec 18, 2015
@tpetr tpetr removed hs_qa labels Dec 18, 2015
@ssalinas ssalinas deleted the fix_search_input branch April 5, 2016 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants