Origin/features/show progress in taskbar for push or pull #8587
Conversation
outofambit
added
electron
There was a problem hiding this comment.
hey @ChamodyaDias thanks for making this work, it looks great. ✨ I'm going to talk with the team to see which release this can go out in, and i'll update here!
|
hi @outofambit thank you reviewing and approving the PR. This is my First Open Source contribution ever! Honestly it is really a small contribution but I'm really glad to help, and will be looking forward for the release. Thanks a lot. |
There was a problem hiding this comment.
Hi @ChamodyaDias, thanks for the PR! @outofambit and I talked about feature-flagging this for beta only initially so we can make sure there aren't any UX pitfalls that we're not thinking about in releasing it more broadly. Here's the documentation for feature flags: https://github.com/desktop/desktop/blob/development/docs/technical/feature-flagging.md#how-to-feature-flag
Are you comfortable making that change, or if not I think @outofambit would be able to knock it out prior to merging. Just let us know!
app/src/lib/stores/app-store.ts
Outdated
| @@ -3362,8 +3362,9 @@ export class AppStore extends TypedBaseStore<IAppState> { | |||
| pushPullFetchProgress, | |||
| })) | |||
| if (pushPullFetchProgress !== null) { | |||
| log.info('setting progress' + pushPullFetchProgress.value) | |||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
app/src/lib/stores/app-store.ts
Outdated
| @@ -3362,8 +3362,9 @@ export class AppStore extends TypedBaseStore<IAppState> { | |||
| pushPullFetchProgress, | |||
| })) | |||
| if (pushPullFetchProgress !== null) { | |||
| log.info('setting progress' + pushPullFetchProgress.value) | |||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@laro67 please stop |
|
Hi @ChamodyaDias, just following up on my previous comment to find out if you'd like to put this behind a feature flag for beta only to start with. Thanks! |
|
@billygriffin Sorry for the delay, and yes of course its totally fine to put this behind feature flags. I'll try to implement it by following the instructions you shared. |
|
@ChamodyaDias Hi! Just following up here, are you planning to feature flag this? Please just let us know what you're thinking, and thank you! 😄 |
ChamodyaDias
dismissed stale reviews from ghost
, ChandyCC, and outofambit
via
fc940a3
…ogress-in-taskbar-for-push-or-pull
|
@billygriffin I've completed the feature flagging. Please check. Thank you. |
This reverts commit dfa434e.
| @@ -21,6 +21,10 @@ function enableDevelopmentFeatures(): boolean { | |||
| return false | |||
| } | |||
|
|
|||
| export function enableProgressBarOnIcon(): boolean { | |||
| return enableDevelopmentFeatures() | |||
There was a problem hiding this comment.
@billygriffin @outofambit I just noticed this feature @ChamodyaDias as a Windows user myself I'm super excited about it. However, I thought it was odd that it is enableDevelopmentFeatures instead of enableBetaFeatures due to the conversations below.
There was a problem hiding this comment.
@say25 yep, that's a fair question! i'm planning to flip this feature flag before our next beta release (probably next week).
There was a problem hiding this comment.
Awesome would be excited/interested in testing and potentially implementing additional Progress Bars for different workflows
There was a problem hiding this comment.
@say25 i'm here for it!!
Closes #8433
Description
Screenshots
Release notes
Notes: