feat(settings): control switching to Output panel when Push-or-deploy… #5904
Conversation
| @@ -19,8 +20,9 @@ import { CompositePostconditionChecker } from './util/compositePostconditionChec | |||
| import { TimestampConflictChecker } from './util/timestampConflictChecker'; | |||
|
|
|||
| export class LibraryDeploySourcePathExecutor extends DeployExecutor<string[]> { | |||
| constructor() { | |||
| constructor(showChannelOutput: boolean = true) { | |||
There was a problem hiding this comment.
This is one area I debated about. 1) passing in the indicator to show the output panel as a constructor argument or 2) creating a separate instance method like setShowChannelOutput() called after LibraryDeploySourcePathExecutor is instantiated.
Option 2 would make the new SfCommandlet<string[]>( command below a little more complicated as you'd have to instantiate LibraryDeploySourcePathExecutor first and then call the method to set the output, then pass the object to the SfCommandlet constructor.
With the current approach, at least TypeScript/Intelisense makes it easier to understand the constructor argument.
Open to thoughts on either approach.
tehnrd
force-pushed
the
deploy-on-save-panel
branch
2 times, most recently
from
f0dc800 to
3b421e0
Compare
|
The implementation looks good to me. @rsensharma would you like to take a look at the proposal? |
|
@tehnrd I was recently looking for a way to turn off the auto focus on the output panel and came across this PR, thank you for contributing to this. Question about the behavior if you have the bottom panel hidden...assuming there are no errors in the save, does the panel stay hidden all together? |
tehnrd
force-pushed
the
deploy-on-save-panel
branch
2 times, most recently
from
3b421e0 to
4ca61d0
Compare
|
This would be a huge QOL improvement. I would suggest 2 changes:
Personally (and I imagine there's a decent overall user split) I don't use deploy-on-save so this actually wouldn't benefit me as currently implemented. Additionally (and I would venture a guess that I'm probably not the only one, although admittedly maybe more likely in the minority here), I don't use the Problems view since I have a chaotic workspace with dozens of files open and therefore a Problems tab with hundreds of warnings/errors at any given time. I actually didn't even realize that deployment errors were surfaced there 😆 . Now that I know that, I could potentially transition to use it instead of the Output tab, though. tl;dr love it, but would like to see more flexibility/options 😄 |
|
@klouf-threshold, I am definitely open to adding that, but I would like someone from Salesforce to provide some input on what they would like to see. |
…-on-save is enabled
tehnrd
force-pushed
the
deploy-on-save-panel
branch
from
4ca61d0 to
0893341
Compare
@mingxuanzhangsfdx As a first step, this looks good. We can add more flexibility in the future. |
|
@tehnrd - We currently don't have the bandwidth to work on this. Your initial use case/implementation looks fine. We can get that added. For adding flexibility and the other use cases, I can provide more clear requirements, if you are up for to do the development. Let me know. fyi - @mingxuanzhangsfdx |
There was a problem hiding this comment.
Looks good! Thanks for the enhancement. If you would like to support more flexibility for the feature, please continue the discussion on Github Issue and tag me or @rsensharma .
|
Amazing, thanks all. Unfortunately, I don't have the bandwidth to expand on this either, but if someone opens a new issue and tags me, I might be able to get to it someday. |
What does this PR do?
When Push-or-deploy-on-save is enabled, do not switch the UI to the Output panel when a file is saved.
What issues does this PR fix or reference?
#5903
#5903 @W-17206953@
Functionality Before
https://www.loom.com/share/a59adaa191474e64a835e9d52121f6c3?sid=36357412-c0f6-48b8-93bd-ea2df8b6f16b
Functionality After
https://www.loom.com/share/a59adaa191474e64a835e9d52121f6c3?t=279&sid=9bc3b0f9-7245-4026-aa22-fd3e64d05300 (same video with timestamp start at functionality after)