Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Added search box to ExtensionManager dialog. #3523

Closed
wants to merge 1 commit into from
Closed

Conversation

njx
Copy link

@njx njx commented Apr 21, 2013

This adds filter-as-you-type functionality to the Extension Manager.

Some notes:

  • Note that the view now has its own model (separate from Extension Manager) that manages the sorting and filtering of items within the view--this really seemed view-specific, not appropriate to go into the central Extension Manager code. The model still relies on the Extension Manager to do the actual fetching of the registry from the server.
  • I didn't break the view model out as a separate file since it's really specific to the view--no one else should need to know much about it. Of course, I already broke that rule since the dialog knows about it :), but I think of the dialog as just a thin shell around the view. I'm happy to break the model out into a separate file if it's appropriate.
  • I also didn't add unit tests for the dialog since the filtering functionality is tested on the model.
  • This just does a full linear search on each keystroke. I added a task to the Trello card to stress-test this with a reasonable number of extensions. If it turns out to be slow, we can back it down to do the search on Enter instead.
  • This doesn't highlight the found search string in each item. I wasn't clear on whether this was part of the criteria--need to check with @adrocknaphobia.
  • The search box is actually in the outer dialog, not the view itself, just because it seemed to make more sense there in the current design. Once we get the final XD design, it's easy to move it wherever it needs to be, since it just calls a filter function on the view's model.

@ghost ghost assigned gruehle Apr 22, 2013

// Filter the view's model when the user types in the search field.
$dlg.on("input", ".search", function (e) {
view.model.filter($(this).val());
Copy link
Member

Choose a reason for hiding this comment

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

You could add a filter() method to the view. This would avoid the need to reach all the way into the model object.

@gruehle
Copy link
Member

gruehle commented Apr 22, 2013

Initial review complete.

@gruehle
Copy link
Member

gruehle commented May 10, 2013

@njx - should this pull request be closed?

@njx
Copy link
Author

njx commented May 10, 2013

Yup, I've changed enough code that it's probably not that useful to keep this open.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants