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

DON-1092 – fix campaign card grid input focus breaking Safari scroll positions #1780

Merged
merged 5 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@
"@angular/platform-server": "^18.2.8",
"@angular/router": "^18.2.8",
"@angular/ssr": "^18.2.8",
"@biggive/components": "^202411151222.0.0",
"@biggive/components-angular": "^202411151222.0.0",
"@biggive/components": "^202411271427.0.0",
"@biggive/components-angular": "^202411271427.0.0",
"@fortawesome/angular-fontawesome": "~0.15.0",
"@fortawesome/free-brands-svg-icons": "^6.6.0",
"@fortawesome/free-solid-svg-icons": "^6.6.0",
Expand Down
22 changes: 20 additions & 2 deletions src/app/explore/explore.component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {DatePipe, isPlatformBrowser} from '@angular/common';
import {Component, HostListener, Inject, OnDestroy, OnInit, PLATFORM_ID} from '@angular/core';
import {DatePipe, isPlatformBrowser, ViewportScroller} from '@angular/common';
import {Component, HostListener, Inject, OnDestroy, OnInit, PLATFORM_ID, ViewChild} from '@angular/core';
import {ActivatedRoute, Router} from '@angular/router';
import { BiggiveCampaignCardFilterGrid } from '@biggive/components-angular';
import {skip, Subscription} from 'rxjs';

import {currencyPipeDigitsInfo} from '../../environments/common';
Expand All @@ -19,15 +20,19 @@ import {HighlightCard} from "../highlight-cards/HighlightCard";
providers: [DatePipe]
})
export class ExploreComponent implements OnDestroy, OnInit {
@ViewChild(BiggiveCampaignCardFilterGrid) cardGrid: BiggiveCampaignCardFilterGrid;

campaigns: CampaignSummary[];
currencyPipeDigitsInfo = currencyPipeDigitsInfo;
loading = false; // Server render gets initial result set; set true when filters change.
/** Whether any non-default search logic besides an order change has been applied. */
searched = false;

private blurredSinceLastMajorScroll = false;
private offset = 0;
private routeParamSubscription: Subscription;
private searchServiceSubscription: Subscription;
private readonly smallestSignificantScrollPx = 250;

beneficiaryOptions: string[] = [];
categoryOptions: string[] = [];
Expand All @@ -47,6 +52,7 @@ export class ExploreComponent implements OnDestroy, OnInit {
private route: ActivatedRoute,
private router: Router,
private pageMeta: PageMetaService,
private scroller: ViewportScroller,
public searchService: SearchService,
@Inject(PLATFORM_ID) private platformId: Object,
) {}
Expand Down Expand Up @@ -124,6 +130,18 @@ export class ExploreComponent implements OnDestroy, OnInit {
}

onScroll() {
const scrollPositionY = this.scroller.getScrollPosition()[1];
if (scrollPositionY < this.smallestSignificantScrollPx) {
// If we're now near the top, reset any previous input blurring as it might be helpful to blur again.
this.blurredSinceLastMajorScroll = false;
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think I'm following the logic here. This has a small scroll position so I guess is a minor scroll? So why does it affect the state of blurredSinceLastMajorScroll?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bdsl The idea is that if you go back to the top, you are more likely to be about to interact with the search fields. So there's a reasonable chance that it will be useful to blur afresh when you're done. I'll add a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This early return may stop some more() calls, but since we do the equivalent on meta-campaign already I can't see that causing major problems.


if (!this.blurredSinceLastMajorScroll) {
this.cardGrid && this.cardGrid.unfocusInputs();
this.blurredSinceLastMajorScroll = true;
}

if (this.moreMightExist()) {
this.more();
}
Expand Down
19 changes: 17 additions & 2 deletions src/app/meta-campaign/meta-campaign.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import {
PLATFORM_ID,
StateKey,
TransferState,
ViewChild,
} from '@angular/core';
import { ActivatedRoute, Router, NavigationEnd, NavigationStart } from '@angular/router';
import { BiggiveCampaignCardFilterGrid } from '@biggive/components-angular';
import {SESSION_STORAGE, StorageService} from 'ngx-webstorage-service';
import {skip, Subscription} from 'rxjs';

Expand Down Expand Up @@ -46,6 +48,8 @@ const endPipeToken = 'timeLeftToEndPipe';
],
})
export class MetaCampaignComponent implements AfterViewChecked, OnDestroy, OnInit {
@ViewChild(BiggiveCampaignCardFilterGrid) cardGrid: BiggiveCampaignCardFilterGrid;

// Campaign ID may be passed instead
@Input({ required: false }) private campaignSlug: string;
// Passed only on the fund-filtered view of this page.
Expand All @@ -63,6 +67,7 @@ export class MetaCampaignComponent implements AfterViewChecked, OnDestroy, OnIni
public title: string; // Includes fund info if applicable.

private autoScrollTimer: number | undefined; // State update setTimeout reference, for client side scroll to previous position.
private blurredSinceLastMajorScroll = false;
private campaignId: string;
private offset = 0;
private routeChangeListener: Subscription;
Expand Down Expand Up @@ -202,14 +207,23 @@ export class MetaCampaignComponent implements AfterViewChecked, OnDestroy, OnIni
}

onScroll() {
if (this.scroller.getScrollPosition()[1] < this.smallestSignificantScrollPx) {
const scrollPositionY = this.scroller.getScrollPosition()[1];
if (scrollPositionY < this.smallestSignificantScrollPx) {
// If we're now near the top, reset any previous input blurring as it might be helpful to blur again.
this.blurredSinceLastMajorScroll = false;

// On return with internal app nav, automatic position seems to be [0,59]
// or so as of Nov '22. So we want only larger scrolls to be picked up as
// donor intervention and to turn off auto-scroll + trigger loading of
// additional campaigns.
return;
}

if (!this.blurredSinceLastMajorScroll) {
this.cardGrid && this.cardGrid.unfocusInputs();
this.blurredSinceLastMajorScroll = true;
}

this.shouldAutoScroll = false;

if (this.moreMightExist()) {
Expand Down Expand Up @@ -396,7 +410,8 @@ export class MetaCampaignComponent implements AfterViewChecked, OnDestroy, OnIni
private listenForRouteChanges() {
this.routeChangeListener = this.router.events.subscribe(event => {
if (event instanceof NavigationStart) {
this.navigationService.saveLastScrollY(this.scroller.getScrollPosition()[1]);
const scrollPositionY = this.scroller.getScrollPosition()[1];
this.navigationService.saveLastScrollY(scrollPositionY);

if (isPlatformBrowser(this.platformId) && this.autoScrollTimer) {
window.clearTimeout(this.autoScrollTimer);
Expand Down
Loading