SF-2864 Fix icon run-off on smaller mobile devices

[josephmyers]

This focuses on both checkers and admin's, with checkers being the primary target.

• Slightly narrowed the book-chapter chooser. I'm aware there's an existing refactor to this, so hopefully it will make this less necessary
• Moved chapter audio next to the book-chapter chooser. This makes a little more sense for checkers, since the chapter audio is near the chapter selection
• Removed circle outline from chapter audio icon
• Moved Add Question button to question nav panel. This is still a logical place for it to live, and it fixes a run-off issue for admin's

---

This change is 

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.55%. Comparing base (9df75c1) to head (3c7a3ca).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2993      +/-   ##
==========================================
- Coverage   82.56%   82.55%   -0.01%     
==========================================
  Files         551      551              
  Lines       31981    31981              
  Branches     5163     5185      +22     
==========================================
- Hits        26405    26402       -3     
- Misses       4820     4822       +2     
- Partials      756      757       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[RaymondLuong3]

Sweet. I had my doubts at first whether I would like the adding question button in the question nav, but it does work there. I say we keep it there. Maybe someone will complain, but at least that is just admins. Nice work

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @josephmyers)

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html line 82 at r1 (raw file):

              @if (chapterHasAudio) {
                <button
                  type="button"

I think the manage audio button should be within the "action-icons" div.

Code quote:

              @if (chapterHasAudio) {
                <button
                  type="button"

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html line 190 at r1 (raw file):

      @if (projectDoc && totalVisibleQuestions() > 0) {
        <div id="question-nav" [ngClass]="{ hide: textboxIsShownMobile || userOpenedChapterAudio }">
          <div class="question-nav-left">

Question-nav-start is the better name since it will be on the right in RTL UI

Code quote:

div class="question-nav-left"

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html line 209 at r1 (raw file):

              <button mat-button type="button" (click)="setQuestionsOverlayVisibility(true)">
                {{ t("view_questions") }}
              </button>

Can we also make this an icon button on small screens? It doesn't seem consistent that one button is an icon and one is a text button on the same menu. To keep it even more consistent, lets also add an icon to the "view questions" button.

Code quote:

              <button mat-button type="button" (click)="setQuestionsOverlayVisibility(true)">
                {{ t("view_questions") }}
              </button>

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-chapter-chooser/book-chapter-chooser.component.scss line 17 at r1 (raw file):

    padding-left: 12px;
    padding-right: 12px;
  }

Nit: I like padding: 0 12px to say the same thing.

Code quote:

  ::ng-deep .mat-mdc-text-field-wrapper {
    padding-left: 12px;
    padding-right: 12px;
  }

[josephmyers]

Reviewable status: 1 of 3 files reviewed, 4 unresolved discussions (waiting on @RaymondLuong3)

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html line 82 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

I think the manage audio button should be within the "action-icons" div.

Do you mean "play-chapter"? "manage-audio" is already in that div.

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html line 190 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Question-nav-start is the better name since it will be on the right in RTL UI

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html line 209 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Can we also make this an icon button on small screens? It doesn't seem consistent that one button is an icon and one is a text button on the same menu. To keep it even more consistent, lets also add an icon to the "view questions" button.

I think an icon is a really good idea. I added one but ended up leaving the full text even on mobile, for two reasons:

• There's plenty of room for it
• It felt a bit too risky to completely remove the text. Users may not immediately understand or be able to find where the questions went/are

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-chapter-chooser/book-chapter-chooser.component.scss line 17 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Nit: I like padding: 0 12px to say the same thing.

The problem is that overwrites the top and bottom paddings, which have their own settings and shouldn't be forced to zero. I only want to touch padding left and right

[RaymondLuong3]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @josephmyers)

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html line 82 at r1 (raw file):

Previously, josephmyers wrote…

Do you mean "play-chapter"? "manage-audio" is already in that div.

Sorry, that is correct. the Play audio button should be in the action-icon div.

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-chapter-chooser/book-chapter-chooser.component.scss line 17 at r1 (raw file):

Previously, josephmyers wrote…

The problem is that overwrites the top and bottom paddings, which have their own settings and shouldn't be forced to zero. I only want to touch padding left and right

That's fine. It makes sense this way too.

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.scss line 97 at r2 (raw file):

    overflow: hidden;
    background: #fff;
    transition: height 0.4s cubic-bezier(0.68, -0.6, 0.265, 1.6); // Snap back effect

The effect is not as smooth in RTL mode when closing the questions. It is not a big deal, but wanted to point it out.

Code quote:

 // Snap back effect

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.scss line 227 at r2 (raw file):

      font-size: 24px;
      height: 24px;
      width: 24px;

You can use md-icon-size mixin from checking-global-vars.scss which should set these values

Code quote:

      font-size: 24px;
      height: 24px;
      width: 24px;

[josephmyers]

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @RaymondLuong3)

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html line 82 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Sorry, that is correct. the Play audio button should be in the action-icon div.

Care to share your thoughts why? I'm interested

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.scss line 97 at r2 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

The effect is not as smooth in RTL mode when closing the questions. It is not a big deal, but wanted to point it out.

I'm not seeing any difference over here. I also don't see how RTL would impact this. The only real difference is animating height vs translateX, but it's such a simple animation I can't see how this would have changed much

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.scss line 227 at r2 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

You can use md-icon-size mixin from checking-global-vars.scss which should set these values

Done. Thanks

[RaymondLuong3]

I noticed that when a book has no questions and we are filtering questions by book, the question navigation is missing on the page. The user is not able to create a question except by navigating to the overview page. Can we update the chapter navigation to show even when no questions exist?

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @josephmyers)

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html line 82 at r1 (raw file):

Previously, josephmyers wrote…

Care to share your thoughts why? I'm interested

Ok, so it looks like you might have done this on purpose and moved the "play audio" icon next to the chapter select. It does seem weird to me that this icon is separated from the other icons. I'd prefer if the location of the icon is moved back with the other icons on the toolbar.

[josephmyers]

Good catch. I played around with it a bit, and I don't see any issues/risks with having it shown when there are no questions. It certainly makes navigating away from these chapters easier.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @RaymondLuong3)

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html line 82 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Ok, so it looks like you might have done this on purpose and moved the "play audio" icon next to the chapter select. It does seem weird to me that this icon is separated from the other icons. I'd prefer if the location of the icon is moved back with the other icons on the toolbar.

I think it would make more sense to the user, as mentioned in the PR description. By the other icons, there's not much context -- it's just another icon for them to figure out. By the chapter, it's easier to associate "playing" with the "chapter"

However, I've reverted it. If it's weird, I'm fine without it

[josephmyers]

Note that my change only affects the navbar for Admin's. I think we should consider showing it for all users, but I want to get your thoughts first

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @RaymondLuong3)

[Nateowami]

User story:

1. As a community checker, I discover that I can filter for questions that I have already answered
2. There aren't any
3. I close the question list
4. Now I can't go back and clear the filter, because the UI toolbar to get to it disappeared

[RaymondLuong3]

I agree with Nathaniel, let's show the nav bar for every role.

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @josephmyers)

---

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html line 82 at r1 (raw file):

Previously, josephmyers wrote…

I think it would make more sense to the user, as mentioned in the PR description. By the other icons, there's not much context -- it's just another icon for them to figure out. By the chapter, it's easier to associate "playing" with the "chapter"

However, I've reverted it. If it's weird, I'm fine without it

Thanks!

[josephmyers]

Nice, fixing bugs in Live

Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @josephmyers)

[RaymondLuong3]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @josephmyers)