Implement a Share modal to the Book page including the 4 share icons already existing #6469
Conversation
…ilippos01/openlibrary into 4578/feature/Single-Icon-Share-Button
for more information, see https://pre-commit.ci
…ilippos01/openlibrary into 4578/feature/Single-Icon-Share-Button
openlibrary/macros/SocialShare.html
Outdated
|
|
||
|
|
||
| <div> | ||
| <p style="display: inline-block" class="cta-section-title">$_('Share this book') |
There was a problem hiding this comment.
Let's avoid using inline CSS. New style rules for the SocialShare.html template can be added here: https://github.com/internetarchive/openlibrary/blob/master/static/css/components/shareLinks.less
openlibrary/macros/SocialShare.html
Outdated
| <div class="popup"> | ||
|
|
||
| <div class="content"> | ||
| <header class="header-share"> | ||
| <p>Share via</p> | ||
| <div class="close"> | ||
| <i class="uil uil-times"></i> | ||
| </div> | ||
| </header> | ||
| <ul class="icons"> | ||
|
|
||
| $for share_link in share_links: | ||
| $ track_tag = share_link['text'].replace(' ', '-') | ||
|
|
||
| <a href="$share_link['url']" class="sansserif large" target="_blank" data-ol-link-track="Share|$track_tag"> | ||
| <img title="Share on $(share_link['text'])" alt="$(share_link['text'])" class="share-link" src="$static_url('images/%s.svg' % share_link['text'].lower())"/> | ||
| </a> | ||
|
|
||
|
|
||
|
|
||
| $# rm special characters from url such as single quote which break js prompt/iframe | ||
| $ clean_url = page_url.replace("'", "\\'") | ||
| $ iframe_html = str(embed_iframe(clean_url)).strip() | ||
| <a class="embed-work-btn" title="$_('Embed this book in your website')" data-ol-link-track="Share|Embed" | ||
| onclick="prompt('Copy embed code to clipboard:', '$iframe_html');"> | ||
| <img alt="Embed icon" class="share-link" src="$static_url('images/embed.png')"> | ||
| </a> | ||
|
|
||
|
|
||
| </ul> | ||
| <p>Or copy link</p> | ||
| <div class="copy-field"> | ||
| <i class="url-icon uil uil-link"></i> | ||
| <input type="text" readonly value="$page_url"> | ||
| <button class="copy-button">Copy</button> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
No need to create a new modal component. Open Library uses Colorbox for our other modals, and it should also be used here. Colorbox takes an element's ID, and displays that element in a modal.
You can look at our book notes modal for an example, here. You'll see that the notes modal component consists of a container with a link and a hidden div containing the modal's content.
In index.js, we load the JS for the notes modal if we find any elements with class notes-modal-link on the page, here. If there are notes-modal-link elements on the page, initNotesModal is called. This calls addClickListeners, which displays the modal on notes-modal-link clicks. All of these functions can be found here for reference.
You can probably place the original share links content inside of a Colorbox modal and solve this issue without much new CSS.
|
|
||
| /* js code for the Share module in the book page | ||
| * Coding By CodingNepal - youtube.com/codingnepal */ | ||
| const viewBtn = document.querySelector('.view-modal'), | ||
| popup = document.querySelector('.popup'), | ||
| close = popup.querySelector('.close'), | ||
| field = popup.querySelector('.copy-field'), | ||
| input = field.querySelector('input'), | ||
| copy = field.querySelector('button'); | ||
|
|
||
| viewBtn.onclick = ()=>{ | ||
| popup.classList.toggle('show'); | ||
| } | ||
| close.onclick = ()=>{ | ||
| viewBtn.click(); | ||
| } | ||
|
|
||
| copy.onclick = ()=>{ | ||
| input.select(); //select input value | ||
| if (document.execCommand('copy')) { //if the selected text copy | ||
| field.classList.add('active'); | ||
| copy.innerText = 'Copied'; | ||
| setTimeout(()=>{ | ||
| window.getSelection().removeAllRanges(); //remove selection from document | ||
| field.classList.remove('active'); | ||
| copy.innerText = 'Copy'; | ||
| }, 3000); | ||
| } | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Much of this will not be relevant when you switch to Colorbox. Whatever JS code is needed to display the Colorbox modal should be moved to a separate file (like Share.js), and imported here if share links exist on the page.
static/css/components/Share.less
Outdated
| @@ -0,0 +1,158 @@ | |||
| @import (less) "less/colors.less"; | |||
There was a problem hiding this comment.
Any styles for share links should be added to the existing shareLinks.less file, here. I suspect that much of this new CSS will not be needed once you switch to Colorbox.
Co-authored-by: jimchamp <jameschamp@acm.org>
Co-authored-by: jimchamp <jameschamp@acm.org>
Co-authored-by: jimchamp <jameschamp@acm.org>
Co-authored-by: jimchamp <jameschamp@acm.org>
Co-authored-by: jimchamp <jameschamp@acm.org>
Co-authored-by: jimchamp <jameschamp@acm.org>
for more information, see https://pre-commit.ci
Co-authored-by: jimchamp <jameschamp@acm.org>
…ilippos01/openlibrary into 4578/feature/Single-Icon-Share-Button
for more information, see https://pre-commit.ci
|
hi @jimchamp, we spotted a missing |
…ilippos01/openlibrary into 4578/feature/Single-Icon-Share-Button
jimchamp
added
On testing.openlibrary.org
|
Thanks! I agree that the icons seem a bit large. I'm deploying this to testing so that more people can see the new changes and give feedback. We'll also be asking for feedback during our next community meeting. Thanks again for all of your work on this PR! |
|
Thanks for working on this, @dbouris and @Philippos01! I'll merge this into an integration branch, and will implement the changes that were discussed today. After a round of user studies, we'll release all of the changes in production. |
…already existing (internetarchive#6469) - Replace share links in book page sidebar with a single link that opens share modal - Create `Share.js` for modal link click listeners
jimchamp
removed
the
On testing.openlibrary.org
Closes #4578
This PR, adds a new feature to the Book page. A share modal is being used to group the 4 share icons in a single popup modal. We have used the icon provided to us by @jimchamp in PR #6410.
Technical
Note that the modal appears in the center of the screen every time it is being called. The proposed change works fine for all the screens (mobile - desktop).
Screenshot
Stakeholders
@jimchamp
Attribution Disclaimer: By proposing this pull request, I affirm to have made a best-effort and exercised my discretion to make sure relevant sections of this code which substantially leverage code suggestions, code generation, or code snippets from sources (e.g. Stack Overflow, GitHub) have been annotated with basic attribution so reviewers & contributors may have confidence and access to the correct context to evaluate and use this code.