feat: Add progress step component #139
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pepicrft
force-pushed
the
progress-step
branch
from
ac1936b to
eb7cf8f
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
pepicrft
force-pushed
the
progress-step
branch
from
18a5ab4 to
81f794f
Compare
|
@pepicrft is this ready for review? Any reason for this being a draft? |
fortmarek
reviewed
Jan 3, 2025
Comment on lines
+19
to
+24
| message: String, | ||
| successMessage: String?, | ||
| errorMessage: String?, | ||
| showSpinner: Bool, | ||
| action: @escaping (@escaping (String) -> Void) async throws -> Void, | ||
| theme: Theme, |
There was a problem hiding this comment.
I'd keep the init for dependency injection and pass the rest of the variables through the run invocation
There was a problem hiding this comment.
I can update this one and the others to follow that, but I don't see much value. The public interface is through an instance of Noora, which then instantiates components and invokes the run. From the testing perspective, there isn't a notable benefit that I can think of.
If you feel strongly about it, I can go ahead with the change.
There was a problem hiding this comment.
I'm fine with the current approach 👍
pepicrft
force-pushed
the
progress-step
branch
from
1e10137 to
35a4bc5
Compare
fortmarek
approved these changes
Jan 28, 2025
| @@ -200,4 +200,25 @@ public struct Noora: Noorable { | |||
| theme: theme | |||
| ).run() | |||
| } | |||
|
|
|||
| public func progressStep( | |||
There was a problem hiding this comment.
This doesn't seem to have been addressed
… enhance test coverage for both scenarios
…updates; add new progress step documentation and demo GIFs
…nventional PR checks
pepicrft
force-pushed
the
progress-step
branch
from
461ac7b to
b20f4d6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL;DR
Added a new progress step component to display step-by-step execution with timing and status indicators.
What changed?
ProgressStepcomponent that shows execution progress with optional spinnerHow to test?
Why make this change?
To provide a consistent way to display step-by-step progress during command execution with visual feedback, timing information, and proper error handling. This improves the user experience by giving clear visibility into long-running operations.