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

Feature/2.1 search within #995

Closed
wants to merge 77 commits into
base: release2.1
from

Conversation

Projects
None yet
8 participants
@jeffreycwitt
Contributor

jeffreycwitt commented Jun 30, 2016

Just sending in an early pull request for search within. I think it should definitely still be considered a work in progress. But I think its worth getting wider feed back and design review before we take in much further.

(Don't be misled by branch name. By 2.1 we simply meant search within work built on top of 2.1 release branch)

jeffreycwitt added some commits Sep 17, 2015

peformed git pull from upstream master; includes bug fix for table of…
… contents ranges that don't include all canvas; i.e. fixes bug seen in Gracilis manuscript display
got searchWithin results window to appear with results and effective …
…linking to change canvas to target of search results. Functions have been embedded into window.js as part of $.Window object, but probably need to be moved to a child object
moved main functionality of search within to $.SearchWithin object to…
… conform to more closely to mirador organization; main functionality continues to be working
changes in searchWithin object to adjust when hits property is presen…
…t and to modify annotation list to show highlighting of search results
adjusted getSearchWithinService function on manifest object to handle…
… array or object as value of service field. function now check context to see if this is the service for searching
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jun 30, 2016

Coverage Status

Coverage decreased (-1.8%) to 43.215% when pulling e82eabb on jeffreycwitt:feature/2.1-search-within into 284ffd5 on IIIF:release2.1.

coveralls commented Jun 30, 2016

Coverage Status

Coverage decreased (-1.8%) to 43.215% when pulling e82eabb on jeffreycwitt:feature/2.1-search-within into 284ffd5 on IIIF:release2.1.

@jbhoward-dublin

This comment has been minimized.

Show comment
Hide comment
@jbhoward-dublin

jbhoward-dublin Jul 5, 2016

Contributor

Just to note: users will in some cases arrive at the viewer after executing a full-text search in a discovery environment; it will be useful to be able to pass search terms from that environment to Mirador so that the search is executed when the manifest is opened ... so, there should be an initialisation parameter that can be assigned search terms.

Contributor

jbhoward-dublin commented Jul 5, 2016

Just to note: users will in some cases arrive at the viewer after executing a full-text search in a discovery environment; it will be useful to be able to pass search terms from that environment to Mirador so that the search is executed when the manifest is opened ... so, there should be an initialisation parameter that can be assigned search terms.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 10, 2016

Coverage Status

Coverage decreased (-1.8%) to 42.442% when pulling 5eb4a81 on jeffreycwitt:feature/2.1-search-within into 8bc3982 on IIIF:release2.1.

coveralls commented Jul 10, 2016

Coverage Status

Coverage decreased (-1.8%) to 42.442% when pulling 5eb4a81 on jeffreycwitt:feature/2.1-search-within into 8bc3982 on IIIF:release2.1.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 30, 2016

Coverage Status

Coverage decreased (-1.8%) to 42.487% when pulling f521f40 on jeffreycwitt:feature/2.1-search-within into c03cc14 on IIIF:release2.1.

coveralls commented Jul 30, 2016

Coverage Status

Coverage decreased (-1.8%) to 42.487% when pulling f521f40 on jeffreycwitt:feature/2.1-search-within into c03cc14 on IIIF:release2.1.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Aug 23, 2016

Coverage Status

Coverage decreased (-1.7%) to 45.859% when pulling 02a8259 on jeffreycwitt:feature/2.1-search-within into 3c84050 on IIIF:release2.1.

coveralls commented Aug 23, 2016

Coverage Status

Coverage decreased (-1.7%) to 45.859% when pulling 02a8259 on jeffreycwitt:feature/2.1-search-within into 3c84050 on IIIF:release2.1.

@dicksonlaw583

This comment has been minimized.

Show comment
Hide comment
@dicksonlaw583

dicksonlaw583 Sep 16, 2016

Contributor

Is this PR still under active development? It would be great to get search API integration into upcoming versions of Mirador, and we at University of Toronto are interested in integrating your additions. If you are ready but the test coverage drop is preventing it from being accepted, please let me know if I can help out with that.

Contributor

dicksonlaw583 commented Sep 16, 2016

Is this PR still under active development? It would be great to get search API integration into upcoming versions of Mirador, and we at University of Toronto are interested in integrating your additions. If you are ready but the test coverage drop is preventing it from being accepted, please let me know if I can help out with that.

@jeffreycwitt

This comment has been minimized.

Show comment
Hide comment
@jeffreycwitt

jeffreycwitt Sep 16, 2016

Contributor

I think we're more or less waiting for a design review and discussion of the best ways to use the side panel. The search panel works as is, but it's just a matter of what its final integration should look like. I think it is definitely a priority for 2.2

Contributor

jeffreycwitt commented Sep 16, 2016

I think we're more or less waiting for a design review and discussion of the best ways to use the side panel. The search panel works as is, but it's just a matter of what its final integration should look like. I think it is definitely a priority for 2.2

@snydman

This comment has been minimized.

Show comment
Hide comment
@snydman

snydman Sep 16, 2016

Collaborator

Yes, this is a high priority. It will need design review and test coverage support. Thanks for the offer to help @dicksonlaw583! Now that M2.1 is released we'll make a firm plan for search at the next Mirador call.

Collaborator

snydman commented Sep 16, 2016

Yes, this is a high priority. It will need design review and test coverage support. Thanks for the offer to help @dicksonlaw583! Now that M2.1 is released we'll make a firm plan for search at the next Mirador call.

@rsinghal

This comment has been minimized.

Show comment
Hide comment
@rsinghal

rsinghal Nov 17, 2016

Collaborator

Closing this so we can delete release2.1

Collaborator

rsinghal commented Nov 17, 2016

Closing this so we can delete release2.1

@rsinghal rsinghal closed this Nov 17, 2016

@jbaiter

This comment has been minimized.

Show comment
Hide comment
@jbaiter

jbaiter Nov 18, 2016

Contributor

Closing this so we can delete release2.1

So what does this mean for the Search API within Mirador? Is the implementation in this PR not going to be merged? Or how can I understand that comment? :-)

Contributor

jbaiter commented Nov 18, 2016

Closing this so we can delete release2.1

So what does this mean for the Search API within Mirador? Is the implementation in this PR not going to be merged? Or how can I understand that comment? :-)

@snydman

This comment has been minimized.

Show comment
Hide comment
@snydman

snydman Nov 18, 2016

Collaborator

@jbaiter - @jeffreycwitt and others submitted this PR as a WIP as a way to stimulate discussion and feedback on how to implement Search Within in Mirador. And indeed what has followed has been a detailed discussion and design process to specify the implementation. The current proposal, I believe, is for a small group of engineers to work on this in early 2017. It remains a high priority, just waiting for an engineering team to mobilize at the right moment to re-implement according to the new specification. https://docs.google.com/document/d/1uvwgAvPGxhM94wYylMItSV6nd39V9uObd82NrjoDI9Y/edit?usp=sharing

Collaborator

snydman commented Nov 18, 2016

@jbaiter - @jeffreycwitt and others submitted this PR as a WIP as a way to stimulate discussion and feedback on how to implement Search Within in Mirador. And indeed what has followed has been a detailed discussion and design process to specify the implementation. The current proposal, I believe, is for a small group of engineers to work on this in early 2017. It remains a high priority, just waiting for an engineering team to mobilize at the right moment to re-implement according to the new specification. https://docs.google.com/document/d/1uvwgAvPGxhM94wYylMItSV6nd39V9uObd82NrjoDI9Y/edit?usp=sharing

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