Bib record browser with 'see also', etc from linked authority headings

Bug #1177810 reported by Lebbeous Fogle-Weekley
26
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

For Evergreen master:

    This feature provides a patron-oriented OPAC interface for browsing
    bibliographic records.

    Users choose to browse by Author, Title, Subject, or Series. They then
    enter a browse term, and the nearest match from a left-anchored search
    on the headings extracted for browse purposes will be displayed in a
    typical backwards/forwards paging display. Headings link to search
    results pages showing the related records. If the browse heading is
    linked to any authority records, and if any *other* authority records
    point to those with "See also" or other non-main entry headings, those
    alternative headings are displayed a linked to a search results page
    showing related bib records related to the alternate heading.

    The counts of holdings displayed next to headings from bibliographic
    records are subject to the same visiibility tests as search. This means
    that the org unit (and copy location group) dropdown on the browse
    interface affects counds, and it further means that whether or not
    you're looking at the browse interface through the staff client makes a
    difference.

There are three commits here:
http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/senator/bib-auth-browse-squash

Sites may want to note the two default-off features described at the bottom of the included release note. These are the 0-9 A-Z shortcut links and the leading-article warning global flag.

tags: removed: pullrequest
Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Regarding the warning on users entering articles:

11:21 < senator> this might need to be an org unit setting instead of a global
                 flag. since with browse, as with search, it's easy to see
                 holdings anywhere in the consortium (hm, didn't think about
                 how this interacts with opac hiding), i was thinking it didn't
                 make a ton of sense to tie the setting to a location. thinking
                 about it some more though, maybe using physical_loc actually
                 does make sense.

There might also need to be more (or clearer text) in the warning itself. Awaiting the results of some site testing on that front.

Revision history for this message
Kathy Lussier (klussier) wrote :

Thanks Lebbeous! A couple of things I noticed off the bat:

opac.browse.warnable_regexp_per_class is in 950.data.seed_values.sql, but doesn't appear to have an upgrade script. Jason ran the upgrade scripts when merging this code, and I'm not seeing the setting in the global settings editor in the staff client.

On our test server, the browse search doesn't seem to be handling the filing indicators properly. As an example, to get to "The Assist," I had to type "the", which brings me here:
http://jasondev.mvlcstaff.org/eg/opac/browse?blimit=10&qtype=title&bterm=the+assist&locg=1

The filing indicators in the 245 field seem to be entered properly. However, I did have to do some reading up on filing indicators to verify this, so it's possible that I'm looking at it wrong.

Otherwise, it's looking pretty good so far. Yay!

Revision history for this message
Jason Stephenson (jstephenson) wrote :

opac.browse.warnable_regexp_per_class is in the upgrade script as far as I can tell. It shows up in the global flags editor for me.

It's value is a regular expression. the label is "Map of search classes to regular expressions to warn user about leading articles."

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Hi Kathy,

Thanks for the early feedback!

For the global flag, it should be in the "YYYY..." upgrade script. I see it there at line 6885?

Re the browse headings and the non-filing headings, there is indeed a missing piece that I need to get into the published branch. I'll fix that right away. A partial reingest of bib records ("partial" because only the browse-related bits need reingested) will be needed to see the effect. I'll provide instructions shortly.

Thanks again

Lebbeous

Revision history for this message
Kathy Lussier (klussier) wrote :

Ahhh. OK - I see where my mistake is on the opac.browse.warnable_regexp_per_class global setting. A dumb mistake that I'm too embarrassed to even admit to on Launchpad. :)

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Hi Kathy and Jason,

I'm also adding this in a new git commit on the feature branch, but for purposes of getting the browse headings to reflect non-filing indicators properly, the follow SQL script should make things right (it could take a few minutes to run, as it includes a partial reingest operation on all bib records).

Thanks!
--
BEGIN;

INSERT INTO config.metabib_field (
    id, field_class, name, label, xpath, format,
    search_field, facet_field, browse_field
) VALUES (
    31, 'title', 'browse', 'Title Proper (Browse)',
    $$//mods32:mods/mods32:titleInfo[not (@type)]/mods32:title$$,
    'mods32', FALSE, FALSE, TRUE
);

update config.metabib_field set browse_field = false where id = 6;
select metabib.reingest_metabib_field_entries(id, true, false, true) from biblio.record_entry;

COMMIT;

Revision history for this message
Ben Shum (bshum) wrote :

Curious, it seems that we've changed the class from search-box to search-tools and in the process we no longer have the surrounding padding for the content to keep them from being too squished between various elements on the page. It's mostly jarring because the basic search (starting point) still uses the old CSS that has the padding, and clicking on either advanced search or browse the catalog links will take you to a page where the padding is not present.

Is there a functional reason for that change, or can we tweak it to add back some padding for visual consistency?

Revision history for this message
Jason Stephenson (jstephenson) wrote :

I had to make the changes in the attached patch this morning to get the YYYY upgrade script to run to completion.

The line with "change" in it looks like a typo/think-o.

The hardcoded id of the metabib_field for title browse conflicts with the id of our lccn metabib_field entry. I don't know why our how, but our metab_field table has ids that are off by 1 at some point. I think we may have added a custom field, but I didn't dig deep enough to check.

It would probably be a good idea in future upgrade scripts not to use hardcoded ids. I always try to avoid them in my SQL scripts when updating things.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Ben,

Is the padding issue you're seeing on Jason's server or on your own? If on your own, are your own customizations in play, and can you provide a link so I can see what you mean in action? I'm sure we can find a fix to maintain the padding.

Jason,

Thanks! I pushed a commit to remove the spurious line with 'change' in it.

Regarding hardcoded IDs, I'd like to stop doing them too, but as far as I know we don't have a way for the i18n infrastructure to work without them. I can't use the hunk of your patch that removes the 31 ID because then the oils_i18n_gettext() marker function thing becomes wrong (to be sure, there are no observable effects of this for English-only sites). But if we've already solved this problem in some commits that can serve as examples I'll look to them for this and future work.

Thanks again all.

Lebbeous

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Ben clubbed me about the head until I saw the padding issue that was plainly in front of me all along, even without any customizations in place. :-) I will fix that.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Ah, yeah. I wasn't thinking of i18n when I made that change to remove the id, so yeah I guess that is an issue.

I wonder if it is too late to the set the counter on metabib.field_entry to 1000 or so in the creation scripts. This would give some room for new mandatory fields added below that id and allow for customizations above?

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Doh! I need an edit button for comments.

It's config.metabib_field that I'm talking about and not the non-existent metabib.field_entry.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

I've pushed three new commits since my last comment here (these new commits may call for squashing before this thing gets the pullrequest tag).

Of the new ones, the first should address the padding/layout issue and fix some paging issues including with the 0-9 shortcut link The next two should fix authority heading display as well as the counting of records linked to authorities.

I appreciate all the help everyone is giving in testing this!

Thanks,

Lebbeous

Revision history for this message
Pasi Kallinen (paxed) wrote :

1) Looking for "dog" on Jason's test server, I see some entries where an ampersand shows up in the title and is shown as &amp;

2) i18n in pager.tt2: off the top of my head, perhaps YAOUS string listing the characters would be easiest? The A-Z might contain other than the 26 letters (eg. Icelandic), or for others it doesn't end at Z... Not to mention cyrillic, etc.

Revision history for this message
Pasi Kallinen (paxed) wrote :

Translating the pager.tt2 well isn't quite so easy as just wrapping the words with l() - I reported a wishlist bug that applies to this too: see bug #1178557

Revision history for this message
Dan Wells (dbw2) wrote :

First, let me say that our librarians (and maybe a few patrons) are going to be very, very happy to have some browse functionality. Kudos to everyone who is making this happen!

Second, a few quick observations:

1) I think we need to give more thought to display of the author relator information (e.g. 'creator'). It needs to be set apart somehow, perhaps in parentheses? For instance, see here: http://jasondev.mvlcstaff.org/eg/opac/browse?blimit=10&qtype=author&bterm=Rand&locg=1
We also are seeing three entries there for Ayn Rand, but I am guessing that to be a data issue. Does that seem likely?

2) I am not sure we should have the letter-row pager widget at all. Unless your collection is very, very small, it isn't going to be useful, and worse, I think it's presence will encourage behavior that is likely to fail, and therefore frustrate. If the paging process was super fast, or we only expected a few hundred items, I would feel differently, but as-is, I don't think it works.

Revision history for this message
Kathy Lussier (klussier) wrote :

In regards to the question on the 'creator' issue, we've started a discussion locally about the need for using the "creator" term at all and about whether we need to separate the creator links from other links where the author is an added entry. I can't say we have a firm consensus here yet, but previous discussions on this same topic have resulted in the opinion that, in the above example, there would be one Ayn Rand entry that would lead to a list that includes titles where she is the creator and those where she is an added entry author.

Like I said, I haven't heard enough from my own people to say this is definitely what we would like to see, but I just wanted to mention it before much more work is done to adjust the "creator" display.

I believe the third entry for Ayn Rand is there because there is a period at the end of her name. It would be great if there were a way to somehow collapse those entries where the only difference is a trailing period or a forward slash. I've seen several other examples of those. Also, is there a way to make this case insensitive so that matching titles that only differ in case are collapsed into one entry? You can see an example of this with the assistant at http://jasondev.mvlcstaff.org/eg/opac/browse?blimit=10&qtype=title&bterm=assistant&locg=1.

Thanks!
Kathy

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Hi all,

I'm setting out to address the open questions and more!

Pasi,

The org unit setting idea for a list of paging characters makes sense to me. And in your second comment, where you mention the difficulty in translation short words, but you name pager.tt2, can I take you to mean browse.tt2? And you must be referring to the terms "Browse by", "for" and "held under" ?

Dan,

Regarding the paging links, if nothing else, don't people want to browse some authors under G by whim sometimes? I dunno.

Dan and Kathy,

How about we just get rid of "creator," and collapse such headings into others that lack the relator term? I think we can do that as a matter of configuration. It should just call for an adjusted browse_xpath column on key rows of config.metabib_field. I've had Mike help me to do the same thing on my own test system recently, but I could get that code ready to become part of that feature.

Kathy,

Regarding non-filing indicators, we can do a little more work to extend our working copy of the MODS32 stylesheet (already "enhanced" for Evergreen's purposes) to capture the non-filing part of other fields that have them (130, 440, 630, 730, 740, 830). The reason this works now for 245 but not for the others is simply that MODS is written as if it only the 245 has any such thing as a non-filing indicator. This will also take some more adjustments to the xpath and browse_xpath columns in config.metabib_field, and then another reingest (possibly a full reingest this time; will consider) will be needed.

Nobody,

The way we're counting visible holdings may be slower than it needs to be. We can add a configurable limit to stop counting at, say, 100 by default and just show "100+" next to authors like Stephen King or subjects like Fiction and get a bit of a speed improvement.

Thanks,

Lebbeous

Revision history for this message
Dan Wells (dbw2) wrote :

Luckily, I can state with certainty that nobody will ever want to browse some authors under G, so we should be alright.

More seriously, imagine I am a user wanting to browse for "John Grisham". I see the inviting "G" link, so I click it. I am not where I need to be, so I hit "Forward >". Five pages worth of clicks later, I have gotten as far as "Gaarder-Juntti, Oona", so I think, "this is pointless", then type in "Grisham" like I probably should have done in the first place.

Given the size of most collections, and the limited nature of alphabetical divisions, this seems like it is going to be the most common experience. Also, if anyone *really* just wants to browse at the start of a letter range, they can just search a single letter like the links do now.

Still, if clicking "G" did more than just search, it could be more useful. For instance, if it would break down the "G" range into meaningful sub-range links of Ga, Gh, Go, Gr, and so on, that would be neat, but it seems out of the scope of this feature. Another way it could be more useful would be a "Page 1 of 784" type navigator (with FF and REW style paging controls), but again, we are likely talking about significant development (and a slightly different paradigm, i.e. a "filter" (which is how similar alphabet controls often function)).

On the one hand, one could argue that the links as they are do /something/, so we should leave them in and let people take them out if they don't like them. On the other hand, if any display element is going to be the default, knowing that many (or most) will not turn these off, we must strongly consider whether the feature in question provides the best overall experience.

P.S. On a completely different note, I think limiting to "100+" for display is common and perfectly acceptable.

Revision history for this message
Pasi Kallinen (paxed) wrote :

Lebbeous @ #18: Oops, yes, browse.tt2, and those terms.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

All,

I have addressed one of the open issues here, that of headings from fields like 730 et al not respecting the NFI. Now they should. There is a new commit on the branch that addresses this.

If you're on a test system (Kathy, Jason S) that already has the commits from the branch to-date, instead of rerunning the YYYY upgrade script from the branch (which would work if you were starting from none of this bib-browse code), you want an "inter-upgrade" process, and I think the attached script will get you there.

I'm still looking into the i18n issue, and I would like to hear any other perspectives on the pager, and finally I'm really interested in whether you all feel it might just be best to make relators ("creator" et al) simply disappear from browse headings altogether.

Thanks

Lebbeous

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

I have now pushed another commit to the branch to address Pasi's concern about the untranslatable short terms: 40550cc.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

It looks like I still should run the SQL from comment #6. I see part of it was commented out in YYYY.*.sql.

I'm just asking to make sure. I was running it last night while my dev system was up and got a sharelock violation.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Hi Jason,

You can do the sql from step #6, or if you haven't and want to skip the extra partial reingest therein, you could skip that and skip the DELETEs related to 'title' and 'browse' from the inter-upgrade script.

Ping me in IRC or here if you need!

Lebbeous

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

A new commit has been pushed to the branch. The browse interface now displays public general notes [1] from authority records when the heading being displayed is related to an authority record through a linked bib record.

[1] http://www.loc.gov/marc/authority/ad680.html

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

A new commit has been pushed to the branch (8843a15) that separates the way headings with non-filing indicators sort from the way they display. In other words, The Nutcracker now looks like "The Nutcracker" but sorts like "Nutcracker", assuming agreeable cataloging.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Another commit (89ce2f8) makes it default behavior not to include relator/role information in author browse headings. This means that trailing "creator" term doesn't appear.

I've applied this to Jason S's test system as well, although there's a large reingest going on, and I don't want to start it over just for this. That means that some browse headings on that system won't be respecting the new configuration, while others will. It can be cleaned later, but this is a just a test system churning through a production-sized catalog.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

I've added another commit (204b1e2) to correct the way tracings from authority records are rendered. Inter-subfield spacing is now better, subfield order is preserved properly, and subfields ‡w, ‡2, ‡4, and ‡5 no longer display by default.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

I've added another commit (58c1422) to correct a failure of the browse code to perceive the link between subject headings and authority records.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Another new commit (cb91cf5) hides "see also" reference headings and the like if there are no way to see what bibs may link to the reference.

This might be abuse of vocabulary, but I called reference headings "tracings" in this commit message that I hope explains it better:

> OPAC Browse: Show authority tracings only when inter-authority
> links exist
>
> This is something of a reversion to an earlier behavior. Now we
> don't display See Also tracings from an authority records linked
> to a bib heading unless that authority record tracing is itself
> linked to another authority record where the same tracing is a
> main entry. Without that authority-to-authority link, we can't say
> whether any bibs are authority-linked to the "see also" term.
>
> Unlike before, the tracings are available to the template even if
> a<->a links aren't there, so a site could make a customization if
> they preferred to see all tracings, linked or not.
>

If that's still confusing, consider the attached diagram (.png image).

Imagine that Twain, Mark and Clemens, Samuel are both authorized headings, and in your catalog you have one bib associated with each authorized author heading. You're browsing authors, and you see Twain, Mark, because that was extracted from the Huckleberry Finn record. There's a Twain, Mark authority, so we can show (or not show) a See Also link to Clemens, Samuel, but we only know whether any bibs are linked to Clemens, Samuel if and only the two authorities are linked together. The link between authorities is constituted by the 500 field with the subfield ‡0. String matching would not be good here.

So if the decision of whether to show See Also references or not is contingent upon the whether the system can see any bibs related to the "see also" term, then you're going to see zilch until the authorities are linked to each other.

I hope this makes it clear enough why See Also references will disappear until the auth records in any demo system are fitted with inter-authority links.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

I have added another commit (a1b1962) that should improve the speed at which browse results are returned from the database when headings are related to a great many visible bibs. This is accomplished with a superpages concept similar to that in Evergreen search, so that we don't keep checking thousands of bibs for visibility when we might as easily just say "at least 100."

The same commit also includes a bugfix to address a problem where the counts of bibs related to an authority reference could disappear on some page loads because they weren't put into memcache properly with the rest of the dataset.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

One more commit (c7eb2e7) to fix a tiny bug in the previous one that broke authority reference links.

Ben Shum (bshum)
Changed in evergreen:
status: New → In Progress
assignee: nobody → Lebbeous Fogle-Weekley (lebbeous)
importance: Undecided → Wishlist
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I get this

psql:Open-ILS/src/sql/Pg/upgrade/YYYY.schema.bib-auth-browse.sql:13: ERROR: cannot ALTER TABLE "control_set_authority_field" because it has pending trigger events

When running the YYYY upgrade script this morning.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

I added a commit in a collab branch to fix the above error:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=939402958030428e8ac0c6042a1f456670c043ec

Had to disable triggers while altering and updating a table.

Dan Wells (dbw2)
Changed in evergreen:
milestone: 2.5.0-m1 → 2.5.0-m2
Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

I've pushed a new commit (0c583d2) to Jason's collab branch (*the* branch for now) that makes some interface/usability improvements. See the commit message for specifics.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

I've pushed another new commit (2a43373) to the collab branch that moves the best-matched entry from the top of the displayed result set to the middle.

This does come with changes to the DB layer, but these changes do not require any reingesting of records. For test sites that already have the previous changes from YYYY.schema.bib-auth-browse.sql installed, the attached script should get you up to date.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

I've added another new commit (91621e6) to the collab branch to simplify the display of public general notes (aka 'scope notes').

Previously, the browse interface would try to make hyperlinks out of the subfields ‡a, but depending on the record that can just get too hairy.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

I've added another new commit (ff053f7) to the collab branch to add a CSS class to the best-matched entry (in the middle of the page of results) when a user enters a term. The CSS class does not stick around after the user pages forward or backward. It's got a bold style by default.

This is a templates-only change.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Bug #1200356 will affect the bib-and-auth browser. The browser uses the same database labels as those that were defined for the authority control sets feature, and the 4XX ones were mislabeled with See Also when they ought to have said see from.

That bug report, which should be considered separately from bib-and-auth browse, has a patch to fix the problem.

Dan Wells (dbw2)
Changed in evergreen:
milestone: 2.5.0-m2 → 2.5.0-alpha1
Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Another commit, 8258037, fixes a spacing typo.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Another commit, ca26966, fixes a problem where having a value in the "hits per page" setting caused no results to be shown instead of the desired number of results.

Revision history for this message
Ben Shum (bshum) wrote :

Adding a quick note for others that we're helping to test this code and discovered a potential issue with the browse search taking very long to return results. In our test server, we've been able to consistently get results back from metabib.browse after roughly 16-17 seconds (which is a long time).

The database query that took that much time, we ended up getting out of our logging was something like:

SELECT * FROM metabib.browse( 'title', 'Harry Potter', '1', NULL, 'f', NULL, '10' ) AS "metabib.browse" ;

So that points to an issue with metabib.browse function taking a long time to run. Will follow up with more information as we explore and work with others on this.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Ben,

Try running this SQL statement,

UPDATE config.global_flag SET value = 50, enabled = TRUE WHERE name = 'opac.browse.holdings_visibility_test_limit';

and then time your call to metabib.browse() again. I'd be interested to know whether that makes a big difference.

Thanks,

Lebbeous

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

16:41 < bshum> senator: Hmm, changed that global flag and it still takes about
               16-17 seconds
16:42 * bshum now realizes he should have noted what the original value of
          that flag was.
16:42 < senator> bshum: it was 100
16:42 < senator> thanks for the test!

Revision history for this message
Mike Rylander (mrylander) wrote :

I've pushed a change to the top of Jason's collab branch that breaks up the internal queries a bit to allow simpler, faster execution based on knowledge we have that the PG planner just, well, can't have. From the commit message:

First, we order browse queries over MBE by (sort_value, value) so we
want to match the unique index to that.

We're only going to use the first few rows of the cursors we build from
the back/forward MBE paging queries, and the embedded GROUP BY defeats
the planners desire to use an index for ordering the rows. So, instead,
we use a simpler core query and gather aggregate data as a secondary,
index-capable query for each MBE row.

----

Also, an intermediate upgrade script is available at http://nox.esilibrary.com/~miker/bib-browse-adjust.sql if you're already up to date on a test system previous to my commit.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Thanks to Mike for making browse faster, and to Ben for helping with testing. In my own tests, everything looks kosher.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Another commit, e6bab3b, reverts the display of results as a numbered list to results as a bulleted list.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Another commit, 88b0b3f, avoids displaying tracings if the target authority record has no bibs linked to it. See the commit message for a somewhat fuller explanation.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Another commit, b93936f. reworks the paging shortcuts in a way that I think will work for most users.

From the commit message:

> OPAC Browse: Replace English pager with customizable pager
>
> I have changed the way the formerly English-only paging shortcuts
> work. There is no longer any need to customize config.tt2. Instead,
> set the org unit setting 'opac.browse.paging_shortcuts' to a string
> containing all the characters that you want used in shortcut links, in
> order.
>
> For English, you might use:
>
> *0-9*ABCDEFGHIJKLMNOPQRSTUVWXYZ
>
> Asterisks are used here to set off shortcuts that are more than one
> character long. Everything outside of asteriks is a one-character
> shortcut.
>
>

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Meant to push commit 5f989bd yesterday, an extra blurb in the release note documentation.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Here's a branch rebased against master and squashed down to 7 commits (we were at 30, and I'm sorry I don't think it neatly reduces to fewer than 7)

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/senator/bib-auth-browse-resquash

Also adding pullrequest tag.

Thanks to everyone for all the testing and feedback and fixes so far!

tags: added: pullrequest
Changed in evergreen:
status: In Progress → Confirmed
Revision history for this message
Kathy Lussier (klussier) wrote :

Thanks to Lebbeous, Mike, Grace and everyone else at ESI who worked so well with us as we've tested and provided feedback on this project. It's been a long haul, but I think we've arrived at a great new feature that I hope the rest of the community will be pleased with. Also, a big thanks to our development partners at Bibliomation, not only for providing financial support for this project, but also for their help with testing, particularly in spotting the slow retrieval time for browse results. Also, thanks to others in the community (Dan Wells, Pasi, and anyone who provided feedback in IRC) for your feedback on the project.

Very happy to add my sign-off on this one and looking forward to building upon this in the future.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kmlussier/bib-auth-browse-resquash

Revision history for this message
Ben Shum (bshum) wrote :

I'll sign off as well and help push this to master, but I just need to note some final quirks with the divs and css that trouble me. I'm doing some investigation into the reason why the searchbar on the basic page moved so far to the left (flush with the left side of the browser) and it looks like we lost some of the CSS for padding there. Looking deeper, it looks like we're reusing div IDs on the same page, specifically <div id="search-wrapper"> is outside of each INCLUDE for searchbar.tt2 but is now also used inside that template too.

Might just need some final cleanup and tidying of the various divs and adjustment of the CSS file for display purposes to bring things back in line.

Otherwise, I'm ready to sign off on the functional parts of this work too. Thanks to everyone! More to follow...

Changed in evergreen:
assignee: Lebbeous Fogle-Weekley (lebbeous) → Ben Shum (bshum)
Revision history for this message
Jason Stephenson (jstephenson) wrote : Re: [Bug 1177810] Re: Bib record browser with 'see also', etc from linked authority headings

Quoting Ben Shum <email address hidden>:

> deeper, it looks like we're reusing div IDs on the same page,
> specifically <div id="search-wrapper"> is outside of each INCLUDE for
> searchbar.tt2 but is now also used inside that template too.
>
> Might just need some final cleanup and tidying of the various divs and
> adjustment of the CSS file for display purposes to bring things back in
> line.

Yeah, it does need cleanup. ids are supposed to be unique. Parsers can
get confused or even refuse to parse a document with multiple elements
with the same id.

Ben, do you want me to take a look at it?

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Well, I've got a commit that removes the extra <div id='search-wrapper'>...</div> pairs, but it does nothing for the css. The search box is still all the way to the left.

Revision history for this message
Jason Stephenson (jstephenson) wrote :
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I've messed around with the css a bit on my development vm. I've come to the conclusion that browse.tt2 should have some changes so that it looks more like searchbar.tt2.

The css probably needs some additional tweaks, but what's on my dev VM looks better than it did:

https://jasondev.mvlcstaff.org/eg/opac/home

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Thank s Ben and Jason for spotting the layout issues!

I've pushed an additional commit (61a5457) to the latest branch (collab/dyrcona/bib-auth-browse-resquash) that I believe addresses the problem. It probably isn't pixel-for-pixel exactly the same padding that was there before, but as some divs have moved around, I think it's visually nice now and close enough.

Look good to you guys?

Thanks again.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

And, lest I neglect it any longer, let me express my thanks to Kathy Lussier and everyone at MassLNC and Bibliomation who drove this forward, as well as to all the community participants who helped to test and improve this along the way!

At this point, I think we're just waiting for a final sign-off on the last commit on working/collab/senator/bib-auth-browse-reresquash (freshly rebased against master; no conflicts; Jason's last layout commit already signed-off by me). Then the 9 commits at the head of the branch are ready for master.

Thanks again!

Dan Wells (dbw2)
Changed in evergreen:
assignee: Ben Shum (bshum) → Dan Wells (dbw2)
Revision history for this message
Dan Wells (dbw2) wrote :

For a variety of reasons, I think this feature will be the highlight of 2.5. I signed off on the last commit and pushed the lot to master. Thanks to everyone who made this happen!

Changed in evergreen:
assignee: Dan Wells (dbw2) → nobody
status: Confirmed → Fix Committed
Ben Shum (bshum)
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.