Clicking Book Page Star Ratings summary goes to ratings section by michas9009 · Pull Request #7125 · internetarchive/openlibrary

michas9009 · 2022-11-06T03:07:05Z

… redirect user to rate section and highlights this section.

Clicking Book Page Star Ratings summary redirect user to rate section and also highlights this section.

Technical

In the file "StarRatings.html", div with class="review" now also has an id="starRatingSection".
In the file "StarRatingsStats.html", there is a change that provides clickable link to "rate" section:
< li onclick="location.href='#starRatingSection';" >

In terms of a design (highlighting), I added detailed comments to the code. (If needed, I can describe changes even more.)

Testing

After picking one of the books and clicking on Book Star Rating, you should be redirected to "rate" section, this section should be also highlighted for a moment.

Screenshot

Stakeholders

@mekarpeles

…Ratings summary redirect user to rate section and highlights this section.

jimchamp

Thanks @michas9009! I love the way the animation looks.

I noticed that the star rating component is hidden beneath our header component after the scroll on mobile and tablet views:

Increasing scroll-margin-top for these views should help.

jimchamp · 2022-11-19T02:13:56Z

openlibrary/macros/StarRatingsStats.html

      <span class="dot">·</span>
  </li>
-  <li>
+  <li onclick="location.href='#starRatingSection';">


Suggested change

<li onclick="location.href='#starRatingSection';">

<li>

The onclick attribute should be added to the previous <li> element, here.

Hey @jimchamp
So I've changed the onclick placement and it works as it should. And I've also changed cursor to "pointer" when you hover over the clickable text (star rating) (picture 1).

Picture 1:

But the issue with scrolling is not happening on my part. I have already applied scroll-margin-top in "rating-form.less" (picture 2)

Picture 2:

So should I do another PR so you can test it again? I've tried changing my browser to mobile version (with lots of different variations of resolutions) and it worked on every one of them.

Also, javascript_test that has failed...is that something that I should fix or be concerned about? (picture 3)

Picture 3:

Sorry for the delayed response!
No need to create another PR. Every push to your branch will also update this PR.
Changing the cursor to a pointer is a great idea --- thanks for doing that!
The javascript tests are failing because rating-form.less is not formatted properly. Most of this can be fixed by running npx stylelint --fix ./**/*.less in the web container. If you run this and see any EACCESS errors, try again after opening bash as the root user:
docker-compose exec -uroot web bash
After running the lint fix script, the color-no-hex linting errors will still exist. You can solve this by replacing the hex color values with @link-blue (this is defined in our colors.less file, here.

michas9009 · 2022-12-03T01:10:00Z

@jimchamp @mekarpeles
Hey, so (I think?) I did another PR and everything should be working now.
I was waiting before I did this PR (I had it ready 2 weeks ago) because I was waiting for any response to my comment above, but I didnt get any :D , so you can review it now and also take a look at that comment.

NEW CHANGES:

clickable link is on the right place now
added cursor to "pointer"

CHANGE THAT WAS REQUESTED (but it was already implemented in my first PR):

star rating component is hidden beneath header component on mobile
- this was already working on my version (I tried multiple resolutions and phone brands) so I'm not really sure, where
the problem is

Anyway, hope that everything works now! :)

jimchamp · 2022-12-05T19:06:45Z

Another PR is not necessary. Please include all requested changes in this PR.
Remaining requested changes:

Use px instead of rem as a measurement unit. You'll likely need to change the value from 10, as well.
Do not use hex values for colors. Instead, use the colors defined in the colors.less file.
Move the scroll-margin-top rule to the .review selector, not .review:target.
Fix all outstanding CSS linting errors.

Please let me know if you have any questions.

michas9009 · 2022-12-08T11:08:35Z

@jimchamp
Hey, I just made the changes and everything should work now.
Let me know if there is any problem.

jimchamp

Thanks @michas9009! Will commit the suggested change and merge this PR.

I think that the html {scroll-behavior: smooth} rule should probably be in work.less. I'll create a PR that makes this change, as I don't want to hold this PR up any longer.

jimchamp · 2022-11-19T02:34:03Z

static/css/components/rating-form.less

+// smooth scrolling and highlighting area after clicking on anchor link
+.review:target {
+  animation: highlight 3s;
+  scroll-margin-top: 10rem;


Suggested change

scroll-margin-top: 10rem;

I'm not sure that this is doing anything here. Try moving it to the .review selector (without the target pseudo class).

Also, please use px instead of rem, which can be difficult to work with given the current state of our CSS.

When testing the current code, you should see a difference when using different browsers (firefox v. chrome).

jimchamp · 2022-11-23T18:33:06Z

openlibrary/macros/StarRatingsStats.html

      <span class="dot">·</span>
  </li>
-  <li>
+  <li onclick="location.href='#starRatingSection';">


Sorry for the delayed response!
No need to create another PR. Every push to your branch will also update this PR.
Changing the cursor to a pointer is a great idea --- thanks for doing that!
The javascript tests are failing because rating-form.less is not formatted properly. Most of this can be fixed by running npx stylelint --fix ./**/*.less in the web container. If you run this and see any EACCESS errors, try again after opening bash as the root user:
docker-compose exec -uroot web bash
After running the lint fix script, the color-no-hex linting errors will still exist. You can solve this by replacing the hex color values with @link-blue (this is defined in our colors.less file, here.

static/css/components/rating-form.less

…ion (#12105) Replaces the "Locate" CTA button (which sent patrons off-site to WorldCat) with a "Check Options" button that keeps patrons on Open Library by navigating to the edition page's vendor/library section via an anchor link. Changes: - LocateButton.html: rename label to "Check Options", update href from `/books/{key}/-/borrow?action=locate` to `/books/{key}#check-options`, remove `target="_blank"` and `cta-btn--external` class, update analytics tracking attribute to `CTAClick|CheckOptions` - databarWork.html: add `id="check-options"` to `.panel.desktop-vendor` so the sidebar vendor/library section is a named anchor target - view.html: add `id="check-options"` to `.panel.mobile-vendor` so the responsive mobile equivalent is also reachable as an anchor target - read-panel.css: add `@keyframes check-options-highlight` glow animation (uses `var(--primary-blue)`, fades out over 3s) and `:target` rules for `.desktop-vendor` and `.mobile-vendor` with `scroll-margin-top: 80px` to clear the sticky header; add `scroll-behavior: smooth` to `html` - borrow.py: remove the dead `action == 'locate'` → WorldCat redirect Approach follows the pattern established in PR #7125 (star ratings scroll/highlight).

Solves the whole Issue internetarchive#7024. Clicking Book Page Star …

48c0eaa

…Ratings summary redirect user to rate section and highlights this section.

mekarpeles assigned jimchamp Nov 8, 2022

jimchamp requested changes Nov 19, 2022

View reviewed changes

jimchamp added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Nov 19, 2022

mekarpeles added the Priority: 2 Important, as time permits. [managed] label Nov 21, 2022

cdrini changed the title ~~Solves the whole Issue #7024. Clicking Book Page Star Ratings summary…~~ Clicking Book Page Star Ratings summary goes to ratings section Nov 28, 2022

Solves additional details on Issue internetarchive#7024.

f921aa1

Solves additional details on Issue internetarchive#7024.

620028b

jimchamp approved these changes Dec 8, 2022

View reviewed changes

Update static/css/components/rating-form.less

3624bbf

jimchamp merged commit 49c9ebf into internetarchive:master Dec 8, 2022

jimchamp mentioned this pull request Dec 8, 2022

Book page star ratings analytics + small refactor #7248

Merged

mekarpeles mentioned this pull request Mar 25, 2026

Update Locate Button to navigate to Book Page section #12105

Open

6 tasks

mekarpeles mentioned this pull request Mar 25, 2026

Redesign Locate button as "Check Options" with on-site anchor navigation #12191

Draft

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Clicking Book Page Star Ratings summary goes to ratings section#7125

Clicking Book Page Star Ratings summary goes to ratings section#7125
jimchamp merged 4 commits intointernetarchive:masterfrom
michas9009:7024/feature/clicking-book-page-starRating-goes-to-rate-section

michas9009 commented Nov 6, 2022

Uh oh!

jimchamp left a comment

Uh oh!

jimchamp Nov 19, 2022

Uh oh!

michas9009 Nov 21, 2022

Uh oh!

jimchamp Nov 23, 2022

Uh oh!

michas9009 commented Dec 3, 2022

Uh oh!

jimchamp commented Dec 5, 2022

Uh oh!

michas9009 commented Dec 8, 2022

Uh oh!

jimchamp left a comment

Uh oh!

jimchamp Nov 19, 2022

Uh oh!

jimchamp Nov 23, 2022

Uh oh!

jimchamp Nov 23, 2022

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

michas9009 commented Nov 6, 2022

Technical

Testing

Screenshot

Stakeholders

Uh oh!

jimchamp left a comment

Choose a reason for hiding this comment

Uh oh!

jimchamp Nov 19, 2022

Choose a reason for hiding this comment

Uh oh!

michas9009 Nov 21, 2022

Choose a reason for hiding this comment

Uh oh!

jimchamp Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

michas9009 commented Dec 3, 2022

Uh oh!

jimchamp commented Dec 5, 2022

Uh oh!

michas9009 commented Dec 8, 2022

Uh oh!

jimchamp left a comment

Choose a reason for hiding this comment

Uh oh!

jimchamp Nov 19, 2022

Choose a reason for hiding this comment

Uh oh!

jimchamp Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

jimchamp Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants