Skip to content
This repository has been archived by the owner on Mar 24, 2020. It is now read-only.

Fix the collection item view #365

Closed
wants to merge 1 commit into from

Conversation

VivianChu
Copy link
Member

Fixes #361 ; refs #361

Fix distorted item view display issue for RDCP collection

Changes proposed in this pull request:

  • Remove f.note_tesim.hl.fragsize for note field
  • Add fixture files and test

@ucsdlib/developers - please review

Copy link
Member

@mcritchlow mcritchlow left a comment

Choose a reason for hiding this comment

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

So this is interesting, in that removing the custom fragsize setting fixes this problem. It looks like we had the MaxFragsize set to 150 while the default from Solr appears to be 100 So the difference of 50 characters is enough to introduce/fix this problem?

@VivianChu
Copy link
Member Author

@mcritchlow - I tried to change the MaxFragsize to 250 before and it didn't fix the problem. However, when I removed it, then it worked.

@VivianChu
Copy link
Member Author

@mcritchlow - attached is the screen shot when that value is set to 250. However, when set it to 300, it also worked. What do you think? Setting it to 300 or remove it?
note-search2

@mcritchlow
Copy link
Member

mcritchlow commented Dec 8, 2017

So after further discussion on the Sprint call, it seems like the issue is that we have embedded html in the Notes that are getting cut off in the highlight snippet, so we end up with opened tags that are never closed. The challenge here is we'll never know exactly where these embedded tags might be in the note string, so I'm a little wary of setting a new fragsize value since it might break things for a different note.

Is is practical to strip out html from highlighted search result fields?

@VivianChu
Copy link
Member Author

VivianChu commented Dec 8, 2017

@mcritchlow - https://github.com/ucsdlib/damspas/blob/develop/app/controllers/catalog_controller.rb#L84 , we can remove that and the tag will use the default value em tag instead and still cause the distorted view.
Or we can remove note field from highlight feature - https://github.com/ucsdlib/damspas/blob/develop/app/controllers/catalog_controller.rb#L95

@mcritchlow
Copy link
Member

@VivianChu - sorry, i mean to say, can we remove the embedded html tags from the note field, not the span wrapper. Looking at the example you mentioned that's linked in the ticket, it looks like it's getting cut off right in the middle of rendering the link:

<span>For the latest snapshot of the float data, see the Related Resource section on the <a href="http://library.ucsd.edu/dc/collection/<span ...

If that's not practical (or we can't think of some other way to better control the html output of the highlighted snippet), then like you said we might have to consider something more significant, like not displaying those at all because there is so much variability in them..

@VivianChu
Copy link
Member Author

@mcritchlow - I'll try other way to remove the embedded html tags from the note field. I'll keep you posted.

@@ -19,7 +19,7 @@ def field_with_highlighting document, field, sep=field_value_separator
#Snippets for no default indexed fields
if(hitsonly && highlight_values != nil && highlight_values.count > 0)
highlight_values.collect! {|m|m.length < blacklight_config.hlMaxFragsize || m.ends_with?(".") ? m : m+ " ..."}
return highlight_values.join(sep).html_safe
return strip_tags(highlight_values.join(sep))

Choose a reason for hiding this comment

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

Layout/Tab: Tab detected.

@@ -19,7 +19,7 @@ def field_with_highlighting document, field, sep=field_value_separator
#Snippets for no default indexed fields
if(hitsonly && highlight_values != nil && highlight_values.count > 0)
highlight_values.collect! {|m|m.length < blacklight_config.hlMaxFragsize || m.ends_with?(".") ? m : m+ " ..."}
return highlight_values.join(sep).html_safe
return strip_tags(highlight_values.join(sep))

Choose a reason for hiding this comment

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

Layout/IndentationConsistency: Inconsistent indentation detected.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 66.599% when pulling ce39923 on feature/collectiton_items_view into 9a65f19 on develop.

Add test

Remove the embedded html tags from the note field

Remove tab

Fix layout issue

Fix space issue

Fix space issue
@VivianChu VivianChu force-pushed the feature/collectiton_items_view branch from ce39923 to 3a81220 Compare December 10, 2017 03:00
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 66.611% when pulling 3a81220 on feature/collectiton_items_view into 9a65f19 on develop.

@VivianChu VivianChu closed this Dec 11, 2017
@VivianChu VivianChu deleted the feature/collectiton_items_view branch December 15, 2017 00:23
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.

None yet

4 participants