You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "scottyaslan (via GitHub)" <gi...@apache.org> on 2024/07/11 16:29:04 UTC
[PR] [NIFI-13519] allow user to choose to enable, disable, or use device s… [nifi]
scottyaslan opened a new pull request, #9076:
URL: https://github.com/apache/nifi/pull/9076
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13519] allow user to choose to enable, disable, or use device s… [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #9076:
URL: https://github.com/apache/nifi/pull/9076#discussion_r1676393678
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/app.module.ts:
##########
@@ -51,6 +55,14 @@ import { loadingInterceptor } from './service/interceptors/loading.interceptor';
import { LoginConfigurationEffects } from './state/login-configuration/login-configuration.effects';
import { BannerTextEffects } from './state/banner-text/banner-text.effects';
+const entry = localStorage.getItem('disable-animations');
+let disableAnimations: string = entry !== null ? JSON.parse(entry).item : '';
+
+// honor OS settings if user has not explicitly disabled animations for the application
+if (disableAnimations !== 'true' && disableAnimations !== 'false') {
+ disableAnimations = window.matchMedia('(prefers-reduced-motion: reduce)').matches.toString();
Review Comment:
I think we can get rid of it in the navigation component.
##########
nifi-frontend/src/main/frontend/apps/nifi-jolt-transform-ui/src/app/app.module.ts:
##########
@@ -30,6 +34,14 @@ import { MAT_FORM_FIELD_DEFAULT_OPTIONS } from '@angular/material/form-field';
import { EffectsModule } from '@ngrx/effects';
import { JoltTransformJsonUiEffects } from './pages/jolt-transform-json-ui/state/jolt-transform-json-ui/jolt-transform-json-ui.effects';
+const entry = localStorage.getItem('disable-animations');
+let disableAnimations: string = entry !== null ? JSON.parse(entry).item : '';
+
+// honor OS settings if user has not explicitly disabled animations for the application
+if (disableAnimations !== 'true' && disableAnimations !== 'false') {
+ disableAnimations = window.matchMedia('(prefers-reduced-motion: reduce)').matches.toString();
Review Comment:
I think we can get rid of it in the navigation component.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13519] allow user to choose to enable, disable, or use device s… [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman merged PR #9076:
URL: https://github.com/apache/nifi/pull/9076
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13519] allow user to choose to enable, disable, or use device s… [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on PR #9076:
URL: https://github.com/apache/nifi/pull/9076#issuecomment-2226175024
Will review...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13519] allow user to choose to enable, disable, or use device s… [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on code in PR #9076:
URL: https://github.com/apache/nifi/pull/9076#discussion_r1676358897
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/app.module.ts:
##########
@@ -51,6 +55,14 @@ import { loadingInterceptor } from './service/interceptors/loading.interceptor';
import { LoginConfigurationEffects } from './state/login-configuration/login-configuration.effects';
import { BannerTextEffects } from './state/banner-text/banner-text.effects';
+const entry = localStorage.getItem('disable-animations');
+let disableAnimations: string = entry !== null ? JSON.parse(entry).item : '';
+
+// honor OS settings if user has not explicitly disabled animations for the application
+if (disableAnimations !== 'true' && disableAnimations !== 'false') {
+ disableAnimations = window.matchMedia('(prefers-reduced-motion: reduce)').matches.toString();
Review Comment:
In the navigation component, we ensure that `matchMedia` is available because attempting to use it. Is that check necessary here?
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/navigation/navigation.component.ts:
##########
@@ -154,4 +156,10 @@ export class Navigation implements OnInit, OnDestroy {
this.storage.setItem('theme', theme);
this.themingService.toggleTheme(!!this.darkModeOn, theme);
}
+
+ toggleAnimations(disableAnimations: string = '') {
+ this.disableAnimations = disableAnimations;
+ this.storage.setItem('disable-animations', this.disableAnimations?.toString());
Review Comment:
With the `disableAnimations` parameter defaulting to empty string, I don't think the optional chaining is needed here.
##########
nifi-frontend/src/main/frontend/apps/nifi-jolt-transform-ui/src/app/app.module.ts:
##########
@@ -30,6 +34,14 @@ import { MAT_FORM_FIELD_DEFAULT_OPTIONS } from '@angular/material/form-field';
import { EffectsModule } from '@ngrx/effects';
import { JoltTransformJsonUiEffects } from './pages/jolt-transform-json-ui/state/jolt-transform-json-ui/jolt-transform-json-ui.effects';
+const entry = localStorage.getItem('disable-animations');
+let disableAnimations: string = entry !== null ? JSON.parse(entry).item : '';
+
+// honor OS settings if user has not explicitly disabled animations for the application
+if (disableAnimations !== 'true' && disableAnimations !== 'false') {
+ disableAnimations = window.matchMedia('(prefers-reduced-motion: reduce)').matches.toString();
Review Comment:
Same question here about whether we should protect the call to `matchMedia`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13519] allow user to choose to enable, disable, or use device s… [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #9076:
URL: https://github.com/apache/nifi/pull/9076#discussion_r1676394975
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/navigation/navigation.component.ts:
##########
@@ -154,4 +156,10 @@ export class Navigation implements OnInit, OnDestroy {
this.storage.setItem('theme', theme);
this.themingService.toggleTheme(!!this.darkModeOn, theme);
}
+
+ toggleAnimations(disableAnimations: string = '') {
+ this.disableAnimations = disableAnimations;
+ this.storage.setItem('disable-animations', this.disableAnimations?.toString());
Review Comment:
I think we can get rid of it in the navigation component.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13519] allow user to choose to enable, disable, or use device s… [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #9076:
URL: https://github.com/apache/nifi/pull/9076#discussion_r1676394975
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/navigation/navigation.component.ts:
##########
@@ -154,4 +156,10 @@ export class Navigation implements OnInit, OnDestroy {
this.storage.setItem('theme', theme);
this.themingService.toggleTheme(!!this.darkModeOn, theme);
}
+
+ toggleAnimations(disableAnimations: string = '') {
+ this.disableAnimations = disableAnimations;
+ this.storage.setItem('disable-animations', this.disableAnimations?.toString());
Review Comment:
Good catch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org