-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Re-show DAISY on books page with ocaid when no read/borrow options #723
Conversation
I haven't checked closely to see if there's a reason that prevents this, but could all the duplicate macros calls be combined into a single one? It's seems like they're in every arm of the |
@tfmorris there's probably a better way to do this, I'm taking a look -- thank you! |
…ng or read options
7966722
to
a82a6ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple tiny non-blocking suggestions and a question. Otherwise, LGTM.
$for current_loan in current_and_available_loans[0]: | ||
$if current_loan['user'] == ctx.user.key: | ||
$ user_loan = current_loan | ||
$ break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION: No space needed, I think, between $
and break
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may be required, otherwise it treats break
as a variable, I think this is a weird quirk (http://webpy.org/templetor)
|
||
$elif (availability == 'borrow_available') or my_turn_to_borrow: | ||
<p> | ||
<a href="$borrow_link" title="Borrow from $contributor" id="borrow_ebook" class="borrow-btn borrow-link"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QUESTION: Do we need to html-escape and/or quote-escape $contributor
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No -- if we ran $:
this would render the block as html. Using $
by itself should safely escape
margin-top: 5px; | ||
flex: 1; | ||
} | ||
#content div.Tools ul.shareLinks-list li a { display: block; text-align: center;} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION: Add a final space before the closing bracket.
Looks much improved! I haven't had a chance to build/test, but from a visual inspection it looks good (for what that's worth). Unfortunately I've timed out for the evening trying to get my dev instance up. |
addresses #721, shows daisy download if there's an ocaid but no lending or read options