Use cover and title endpoints for IA cover upload #11077
Conversation
|
@RayBB Originally I had this as a draft because one change was failing tests, but after looking back at it I think the change made by pre-commit fixes it. If I'm misunderstanding this please let me know, but otherwise I think it is good to be reviewed. |
|
@RayBB Have you had a chance to look at this yet? |
There was a problem hiding this comment.
@emggeorge thanks for your patience and following up.
Your changes are a really good start.
I went ahead and pushed up two changes (please review them):
- Using a for loop to generate the images and buttons
- Adding some css for a cap and centering so it looks a little more in line with our usual sytles.
We typically don't have inline styles (style="") in the html. On this particular page you may have noticed we do have height hardcoded to stop the page from jumping around before the css loads. So, we should move your styles to cbox.less. Unfortunately, when I tried doing that the changes weren't reflecting in the UI after running make css and refreshing. So... I asked about it in Slack here. Hopefully someone will help uncover why this is. And if we don't find out soon I'll bring it up on the Tuesday call.
All that is to say, I don't think there's anything else for you to do here at the moment. But changes will be needed when we find out the problem.
|
@RayBB Thanks for your help and explanations! I'll keep those edits in mind next time I work on similar issues. |
Addresses part of the feedback on this pull request. It is a part of the solution for issue #10933
Gets covers from cover and title endpoints from Internet Archive instead of specifying
default.jpg.Example:
Technical
Testing
Screenshot
Stakeholders
@RayBB