Skip to content
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

Exclude materials with too small covers from Related Materials app #100

Conversation

kasperg
Copy link
Contributor

@kasperg kasperg commented Jul 5, 2020

We require a certain size of the cover images we retrieve. If the
image is not large enough it may not display right on certain
devices. Consequently they should be removed for now.

Without this a related material will have a cover image with null as
src. This is just displayed as a blank space.

For good measure we also update the JSDoc to show that the url can
be null.

Before

Apps | Related materials - Entry ⋅ Storybook 2020-07-05 22-21-41

After

Apps | Related materials - Entry ⋅ Storybook 2020-07-05 22-21-17

This is the case if a cover exists for the object but the original is
smaller than the requested size.
@codecov
Copy link

codecov bot commented Jul 5, 2020

Codecov Report

Merging #100 into master will increase coverage by 0.21%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
+ Coverage   80.81%   81.02%   +0.21%     
==========================================
  Files          47       47              
  Lines         641      643       +2     
  Branches      151      148       -3     
==========================================
+ Hits          518      521       +3     
+ Misses        112      111       -1     
  Partials       11       11              
Impacted Files Coverage Δ
src/core/CoverService.js 92.85% <ø> (ø)
...apps/related-materials/related-materials.entry.jsx 90.76% <100.00%> (+1.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c6bd06...adacbb4. Read the comment docs.

We require a certain size of the cover images we retrieve. If the 
image is not large enough it may not display right on certain
devices. Consequently they should be removed for now.

Without this a related material will have a cover image with null as
src. This is just displayed as a blank space.

Expand tests accordingly.
@kasperg kasperg force-pushed the related-materials-filter-covers branch from 5af93ce to adacbb4 Compare July 5, 2020 21:06
Copy link
Contributor

@matis-dk matis-dk left a comment

Choose a reason for hiding this comment

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

Very nice !

@kasperg kasperg merged commit 93950fd into danskernesdigitalebibliotek:master Jul 7, 2020
@kasperg kasperg deleted the related-materials-filter-covers branch July 7, 2020 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants