search

projectblacklight / blacklight

Blacklight provides a discovery interface for any Solr (http://lucene.apache.org/solr) index.
http://projectblacklight.org/
Other
760 stars 256 forks source link

touch up add/remove bookmark interface #298

Closed MrDys closed 12 years ago

MrDys commented 12 years ago

CODEBASE-285: This is NOT the major Bookmarks overhaul contemplated.

Instead, it's just a touch up of add/remove bookmark functionality, to make it more like Folder.

  1. Button should toggle between 'add bookmark' and 'remove bookmark', like Folder does. (Right now it toggles between action 'add' button, and narrative 'this item is in your bookmarks', without action, taking up a lot of space).
  2. Ajaxify (with proper fallback to non-js)
  3. Bookmark toggle should show up on item show page too, like Folder does. Right now it's just on search results.
  4. Clean up CSS on search results a bit, it's still kind of wonky -- I think a simple float would do better than trying to use yui-grids here, as it's currently doing.

Starting on these, I notice some really weird and crufty and hard to use code in Bookmarks. Probably cause it was written a while ago, hasnt' been used by many customers, and hasn't really been touched. There are typos not caught by tests, and code in if/else paths that can never be called, etc. I will try to clean up a bit as I come accross it, as needed for my changes or as I have time.

Will commit the above 1-4 one by one as I do them, but always keeping master in a usable state.

MrDys commented 12 years ago

Original reporter: jrochkind

MrDys commented 12 years ago

jrochkind: New additional plans from discussion at code4lib, and looking at Bill Dueber's vufind interface.

  1. Rip out 'search inside bookmarks' and tagging from bookmarks -- don't work right anyway, have not been maintained, we don't think anyone is actually using them. Keep only a very limited bookmarks feature.
  2. The JS interface for both Bookmarks and Folder should be a checkbox, not a button. (This is actually EASIER to do in a robust way with ajax than our current interface)
  3. Change user interface name of 'Folder' to 'Selected Items'.
  4. Add/Remove Bookmark only appear on item detail screen, not search results screen.

Some people wanted to remove Bookmarks entirely, but as for now not the plan.

MrDys commented 12 years ago

jrochkind: Okay, did all that. Bookmarks is pared down to basics.

Both Bookmark and Folder actions now use ajaxy checkbox instead of a submit button which has a label change.

CSS is changed to be more consistent and clear.

Both Bookmark and Folder now show up on both show and index -- mainly because I wanted to be able to test for it working on both places. I expect/recommend that most people will want to keep Bookmarks off the search results screen; easy to control which of these actions (if any) are on each page by over-riding the new helpers.

There IS still some potential odd behavior with ajaxy bookmarks and folder involving cached copies of page in browser, and state on page not matching actual current state. This is not new, this existed in the old ajax Folder implementation too, and indeed exists in a non-AJAX version too. However, currently actions at least will always do what you say -- a 'remove' action (whether ajax or not) will always cause the item to no longer be in set, even if it wasn't in set before either (that is, it's idempotent), you can't get in a state where you're doing the opposite of what you think you're doing. The possible (not new) bug is an item might be displayed as in set when it's not, or vice versa. Additionally Selected Items count can get out of sync if you use the browser back button to go back to cached version of a page -- that's not new either.

MrDys commented 12 years ago

jrochkind: Oh, and in user interface "Selected Items" now instead of "Folder"