Skip to content

Commit

Permalink
changed covers to use class instead of id (#9565)
Browse files Browse the repository at this point in the history
* changed covers to class instead of id

* Fix: don't repeat element `id` values in covers template

This commit simply passes through an optional value to
`covers/change.html` so that it's possible to avoid repeating the
element `id` values, which allows the "correct" HTML (i.e. the desktop
or mobile HTM) to be edited when the cover popup appears.

On the author page, `covers/change.html` is rendered twice by way of
`authors/infobox.html` being rendered twice in `author/view.html. This
meant element `id` values that should be unique were duplicated. This
broke author photo uploads.

The uploads were fixed in 5495b7b,
however we still had some `id` values repeating, and this meant
regardless of whether one was on mobile or desktop, the element `id`
associated with the mobile rendering was used, because it appeared first
in the document.

Note: this may be somewhat of a moot issue as cover upload on mobile
require one magically knows to click/tap near the bottom of a cover
image to bring up the edit modal.

See 17538fd for more.

---------

Co-authored-by: Scott Barnes <[email protected]>
  • Loading branch information
deysandip301 and scottbarnes authored Jul 22, 2024
1 parent 44a46e2 commit b010f39
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 18 deletions.
14 changes: 7 additions & 7 deletions openlibrary/plugins/openlibrary/js/covers.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@ export function initCoversChange() {
$('.coverPop')
.on('click', function () {
// clear the content of #imagesAdd and #imagesManage before adding new
$('#imagesAdd').html('');
$('#imagesManage').html('');
$('.imagesAdd').html('');
$('.imagesManage').html('');
if (doc_type_key === '/type/work') {
$('#imagesAdd').prepend('<div class="throbber"><h3>$_("Searching for covers")</h3></div>');
$('.imagesAdd').prepend('<div class="throbber"><h3>$_("Searching for covers")</h3></div>');
}
setTimeout(function () {
// add iframe to add images
add_iframe('#imagesAdd', add_url);
add_iframe('.imagesAdd', add_url);
// add iframe to manage images
add_iframe('#imagesManage', manage_url);
add_iframe('.imagesManage', manage_url);
}, 0);
})
.on('cbox_cleanup', function () {
$('#imagesAdd').html('');
$('#imagesManage').html('');
$('.imagesAdd').html('');
$('.imagesManage').html('');
});
}

Expand Down
4 changes: 2 additions & 2 deletions openlibrary/templates/authors/infobox.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
$def with (page, edit_view: bool = False)
$def with (page, edit_view: bool = False, imagesId=0)

$ is_librarian = ctx.user and (ctx.user.is_librarian() or ctx.user.is_super_librarian() or ctx.user.is_admin())

Expand All @@ -17,7 +17,7 @@
<div class="infobox">
<div class="illustration">
$:render_template("covers/author_photo", page)
$:render_template("covers/change", page, ".bookCover img")
$:render_template("covers/change", page, ".bookCover img", imagesId)
</div>
<p class="short-description">
$if wikidata:
Expand Down
14 changes: 7 additions & 7 deletions openlibrary/templates/covers/change.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
$def with (doc, cover_selector=".bookCover img")
$def with (doc, cover_selector=".bookCover img", imagesId=0)

$# If user is not logged-in don't show add/manage link.
$if not ctx.user:
Expand Down Expand Up @@ -28,29 +28,29 @@
$ size = "smallest"

<div class="$size sansserif manageCoversContainer hidden--nojs">
<a aria-controls="addImage" class="coverPop dialog--open">
<a aria-controls="addImage-$imagesId" class="coverPop dialog--open">
<div class="manageCovers" data-config="$dumps({'key': doc.type.key, 'url': get_coverstore_public_url(), 'selector': cover_selector, 'add_url': add_url, 'manage_url': manage_url})">$change</div>
</a>
</div>

<div class="hidden">
<div class="floater" id="addImage">
<div class="floater" id="addImage-$imagesId">
<div class="floaterHead">
<h2>$title</h2>
<a class="dialog--close">&times;<span class="shift">$_("Close")</span></a>
</div>

<div id="tabsImages" class="ol-tabs tabsPop">
<ul>
<li><a href="#imagesAdd">$_('Add')</a></li>
<li><a href="#imagesManage">$_('Manage')</a></li>
<li><a href="#imagesAdd-$imagesId">$_('Add')</a></li>
<li><a href="#imagesManage-$imagesId">$_('Manage')</a></li>
</ul>

<div id="imagesAdd">
<div class="imagesAdd" id="imagesAdd-$imagesId">
$# add image iframe is added here when the popup is loaded
</div>

<div id="imagesManage">
<div class="imagesManage" id="imagesManage-$imagesId">
$# manage images iframe is added here when the popup is loaded
</div>
</div>
Expand Down
4 changes: 2 additions & 2 deletions openlibrary/templates/type/author/view.html
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ <h6 class="collapse black uppercase">$_("Location")</h6>
$page.location
</div>
<span class="mobile-only">
$:render_template("authors/infobox", page)
$:render_template("authors/infobox", page, imagesId="mobile")
</span>
<div class="clearfix"></div>
<div id="works" class="section">
Expand Down Expand Up @@ -153,7 +153,7 @@ <h2 class="collapse">
</div>
<div class="contentOnethird">
<span class="desktop-only">
$:render_template("authors/infobox", page)
$:render_template("authors/infobox", page, imagesId="desktop")
</span>
$def render_subjects(label, subjects, prefix):
$if subjects:
Expand Down

0 comments on commit b010f39

Please sign in to comment.