-
Notifications
You must be signed in to change notification settings - Fork 184
Disable TextAnimation in static contexts #5945
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
base: shauns/06-02-use_rendersingletask_in_app_dev_
Are you sure you want to change the base?
Disable TextAnimation in static contexts #5945
Conversation
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
e41b3b0
to
9cdbf7a
Compare
13bc6e0
to
6f927cf
Compare
9cdbf7a
to
6ded782
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can I reproduce the problem in main? I'm trying with p shopify app deploy --force > output.txt
, but I can't see the loading bar anywhere 🤔
6f927cf
to
97154be
Compare
6ded782
to
1203770
Compare
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationspackages/cli-kit/dist/private/node/ui/hooks/use-exit-on-ctrl-c.d.ts/**
* This hook will cause the process to exit when the user presses Ctrl+C.
*/
export declare function useExitOnCtrlC(): void;
packages/cli-kit/dist/private/node/ui/components/LoadingBar.d.tsimport React from 'react';
interface LoadingBarProps {
title: string;
noColor?: boolean;
}
declare const LoadingBar: ({ title, noColor }: React.PropsWithChildren<LoadingBarProps>) => JSX.Element;
export { LoadingBar };
packages/cli-kit/dist/private/node/ui/components/SingleTask.d.tsimport React from 'react';
interface SingleTaskProps {
title: string;
taskPromise: Promise<unknown>;
noColor?: boolean;
}
declare const SingleTask: ({ taskPromise, title, noColor }: React.PropsWithChildren<SingleTaskProps>) => JSX.Element | null;
export { SingleTask };
Existing type declarationspackages/cli-kit/dist/public/node/ui.d.ts@@ -330,6 +330,21 @@ interface RenderTasksOptions {
* Installing dependencies ...
*/
export declare function renderTasks<TContext>(tasks: Task<TContext>[], { renderOptions }?: RenderTasksOptions): Promise<TContext>;
+/**
+ * Awaits a single promise and displays a loading bar while it's in progress. The promise's result is returned.
+ * @param options - Configuration object
+ * @param options.title - The title to display with the loading bar
+ * @param options.taskPromise - The promise to track
+ * @param renderOptions - Optional render configuration
+ * @returns The result of the promise
+ * @example
+ * ▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
+ * Loading app ...
+ */
+export declare function renderSingleTask<T>({ title, taskPromise }: {
+ title: string;
+ taskPromise: Promise<T> | (() => Promise<T>);
+}, { renderOptions }?: RenderTasksOptions): Promise<T>;
export interface RenderTextPromptOptions extends Omit<TextPromptProps, 'onSubmit'> {
renderOptions?: RenderOptions;
}
packages/cli-kit/dist/private/node/ui/components/TextAnimation.d.ts@@ -2,9 +2,10 @@ import React from 'react';
interface TextAnimationProps {
text: string;
maxWidth?: number;
+ isStatic?: boolean;
}
/**
* applies a rainbow animation to text.
*/
-declare const TextAnimation: React.MemoExoticComponent<({ text, maxWidth }: TextAnimationProps) => JSX.Element>;
+declare const TextAnimation: React.MemoExoticComponent<({ text, maxWidth, isStatic }: TextAnimationProps) => JSX.Element>;
export { TextAnimation };
\ No newline at end of file
|
WHY are these changes introduced?
Disable loading bar animation in cloud environments and non-TTY contexts to improve CLI behavior in automated environments.
Without this, something like
shopify app deploy <opts> &> log.txt
will include lots of renders of the rainbow/hill graphic. With this, it's rendered once and not animated, making the log a lot easier to work withWHAT is this pull request doing?
This PR disables the animated loading bar when running in cloud environments or non-TTY contexts. It:
isStatic
prop to theTextAnimation
componentLoadingBar
component to detect cloud environments and non-TTY contextsTextAnimation
component to disable animation when appropriateHow to test your changes?
command > output.txt
) to test non-TTY behaviorMeasuring impact
How do we know this change was effective? Please choose one:
Checklist