Skip to content
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

Added yearly goal affordances to mobile view in my books page #10208

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sumana-2705
Copy link

Closes #10168

Fix

At present the UI does not have the option to set the yearly goals in Mobile view (as it is present in the side bar which is not included in the mobile view). This PR adds the option to set the yearly goals in the main body in mobile view.

Testing

  1. Deploy the branch locally.
  2. Go to my books page.
  3. Look on the "set yearly goals" section.
  4. Observe the responsiveness in mobile and laptop view.

Screenshot

WhatsApp Image 2024-12-27 at 5 52 04 PM

After setting a particular goal
WhatsApp Image 2024-12-27 at 5 52 04 PM (1)

In laptop view
WhatsApp Image 2024-12-27 at 5 53 18 PM

Stakeholders

@jimchamp @mekarpeles

Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are accessibility issues in these changes.

$ year_markup = year_span(year)
<a class="set-reading-goal-link li-title-desktop" data-ol-link-track="MyBooksLandingPage|SetReadingGoal"href="javascript:;">
$:_('%(year_span)s reading goal', year_span=year_markup)
<img class="icon-link__image li-count" src="/static/images/icons/right-chevron.svg">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This image is missing a text alternative. This is a problem for people using screen readers.

@sumana-2705
Copy link
Author

sumana-2705 commented Dec 29, 2024

@mekarpeles @jimchamp

I have done the changes and the set yearly goals options functions well on smaller screens too. But when we click on the set yearly goals in mobile, the dialog box does not appear in smaller screen and reappears as soon as we increase the screen size. I am working to figure out and correct this. Can anyone please help how to resolve this issue.

VID_20241229_205845.mp4

@jimchamp
Copy link
Collaborator

Part of the problem is that there is only expected to be one dialog with id yearly-goal-modal on the page at a single time. Whenever this link is clicked in mobile views, the desktop dialog is opening. The desktop dialog cannot be seen as it is nested within a div that has display: none for small screen sizes.

To ensure that there is only one #yearly-goal-modal dialog on the screen, you can add the following code here:

  $if mb.is_my_page:
    $ reading_goal_form = render_template('check_ins/reading_goal_form', year=current_year())
    $:render_template('native_dialog', 'yearly-goal-modal', reading_goal_form, title=_('Yearly Reading Goal'))

Then, remove the other reading goal form native dialogs from sidebar.html and mybooks.html.


You will run into a similar issue when a new goal is submitted. The view is updated by the fetchProgressAndUpdateView function, which updates a given yearlyGoalElem element with HTML fetched from the server. Since there will now be multiple progress bars on the page, multiple yearlyGoalElem elements will need to be updated (even though only one will be visible at a time, depending on screen size).

To fix this, you'll likely want to update this line to use document.querySelectorAll('.yearly-goal-section') to get all references to these elements, then update fetchProgressAndUpdateView such that it updates multiple elements with the progress bar. Don't forget to pluralize variable and parameter names where appropriate.

@jimchamp jimchamp added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add yearly reading goals to mobile My Books page
2 participants