Skip to content
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

feat(scm): add visibility variable in plugin API #11412

Merged
merged 1 commit into from
Jul 13, 2022

Conversation

xai
Copy link
Contributor

@xai xai commented Jul 12, 2022

What it does

Adds support to specify whether a SourceControlInputBox is visible

Contributed on behalf of STMicroelectronics

Signed-off-by: Olaf Lessenich olessenich@eclipsesource.com

Fixes #11140
Closes #11348

How to test

  1. Remove the @theia/git extension from your build to correctly test this.
  2. Install the test extension vscode-extension-sourcecontrol-inputbox-11140-0.0.1.zip in your theia workspace from your local filesystem.
  3. Use the command SourceControlInputBox: Status which is provided by the extension to display the properties of sourceControl.inputBox. Verify that a Boolean visible property is present.
  4. (Optional: Use the provided commands SourceControlInputBox: Hide and SourceControlInputBox: Show to manipulate the respective Boolean. Verify the change with SourceControlInputBox: Status.)
  5. This extension can be installed in VSCode as well to compare the behaviour.

Review checklist

Reminder for reviewers

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've modified your PR message (added a test step 0 and that it closes another PR), I hope that's fine with you.

I've also noticed that the changes in the PR don't lead to the actual input box being made visible/invisible:

2022-07-12.11-28-14.mp4

I believe there might be some code missing at this place:

<TextareaAutosize
className={`${ScmCommitWidget.Styles.INPUT_MESSAGE} theia-input theia-scm-input-message-${validationStatus}`}
id={ScmCommitWidget.Styles.INPUT_MESSAGE}
placeholder={message}
spellCheck={false}
autoFocus={true}
value={input.value}
onChange={this.setInputValue}
ref={this.inputRef}
rows={1}
maxRows={6} /* from VS Code */>
</TextareaAutosize>

@xai
Copy link
Contributor Author

xai commented Jul 12, 2022

I've modified your PR message (added a test step 0 and that it closes another PR), I hope that's fine with you.

Thanks!

I've also noticed that the changes in the PR don't lead to the actual input box being made visible/invisible

I'll look into this - thanks for the pointer!

@vince-fugnitto vince-fugnitto added vscode issues related to VSCode compatibility scm issues related to the source control manager labels Jul 12, 2022
@xai
Copy link
Contributor Author

xai commented Jul 12, 2022

I added a change so that the TextareaAutosize element is only included when it is supposed to be visible. SourceControlInputBox: Hide and SourceControlInputBox: Show from the test extension do now have an effect on whether the inputBox is displayed or not.

Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the recent changes, the intended functionality appears to be in place. Some minor comments on the React side of things.

packages/scm/src/browser/scm-commit-widget.tsx Outdated Show resolved Hide resolved
packages/scm/src/browser/scm-commit-widget.tsx Outdated Show resolved Hide resolved
Co-authored-by: Olaf Lessenich <olessenich@eclipsesource.com>

Contributed on behalf of STMicroelectronics

Signed-off-by: Olaf Lessenich <olessenich@eclipsesource.com>
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the changes look good to me. I can confirm that the SourceControlInputBox#visible property behaves as in vscode 👍

@msujew msujew merged commit 0a3b1a4 into eclipse-theia:master Jul 13, 2022
@vince-fugnitto vince-fugnitto added this to the 1.28.0 milestone Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scm issues related to the source control manager vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[vscode] Support property SourceControlInputBox#visible
5 participants