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

CHE-3583: Add new line to message area in Git commit window when Enter pressed #4280

Merged
merged 8 commits into from
Mar 10, 2017

Conversation

vinokurig
Copy link
Contributor

What does this PR do?

Add possibility to insert new lines to 'message' text area in Git commit window

What issues does this PR fix or reference?

#3583

Changelog

Added possibility to insert new lines to 'message' text area in Git commit window by pressing ENTER key

Release Notes

Added possibility to insert new lines to 'message' text area in Git commit window by pressing ENTER key:
commit

Docs PR

N/A Git commit window doesn't mentioned in docs

@vinokurig vinokurig added the kind/enhancement A feature request - must adhere to the feature request template. label Mar 1, 2017
@vzhukovs
Copy link
Contributor

vzhukovs commented Mar 1, 2017

-1 for realization.

It would be better to extend existed com.google.gwt.user.client.ui.TextArea and generalize behavior in widget's constructor. Smth like this:

public class NewTextArea extends TextArea {
    public NewTextArea() {
        super();

         new KeyboardNavigationHandler(this) {
            @Override
            public void onEnter(NativeEvent evt) {
                super.onEnter(evt);

                int cursorPos = getCursorPos();
                setText(new StringBuilder(getText()).insert(cursorPos, '\n').toString());
                setCursorPos(cursorPos + 2);
            }
        };
    }
}

then annotate

    @UiField(provided = true)
    TextArea message;

and create a new instance of message in constructor:

    ...
    @Inject
    protected CommitViewImpl(GitResources res, GitLocalizationConstant locale) {
        ...
        message = new NewTextArea();
    ...

The main pros from this approach is to reuse text area which handles enter key in other components.

@codenvy-ci
Copy link

@vinokurig
Copy link
Contributor Author

@vzhukovskii Thanks for your advice, done.

@vinokurig vinokurig requested a review from vzhukovs March 2, 2017 14:35
@codenvy-ci
Copy link

Build finished.
Build success. $BUILD_URL

@codenvy-ci
Copy link

*
* @author Igor Vinokur
*/
public class TextArea extends com.google.gwt.user.client.ui.TextArea {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use same name in class hierarchy. Maybe it would be better to rename this class smth to related with multiline. e.g. MultilineTextArea or LFTextArea (from https://en.wikipedia.org/wiki/Newline)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to ShiftableTextArea WDYT?

@codenvy-ci
Copy link

Build finished.
Build success. $BUILD_URL

@codenvy-ci
Copy link

public ShiftableTextArea() {
super();

new KeyboardNavigationHandler(this) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this code from constructor?
This looks weird and too complicated for reading

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@codenvy-ci
Copy link

Build finished.
Build # 2126 - FAILED

Please check console output at $BUILD_URL to view the results.

@codenvy-ci
Copy link

Build # 2126 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/2126/ to view the results.

@vinokurig
Copy link
Contributor Author

Selenium tests result:

[TEST]
[TEST] RESULTS ANALYSE:
[TEST]
[TEST] Command line: ./webdriver.sh --analyse 247
[TEST]
[TEST] CI results https://ci.codenvycorp.com/view/qa/job/che-integration-tests/247/
[TEST] 	- Failed:	81
[TEST]
[TEST] Local results:
[TEST] 	- Run:		0
[TEST] 	- Failed:	15
[TEST] 	com.codenvy.ide.codeserver.CheckMainFeatureForLanguageTest.checkLaunchingCodeserver
[TEST] 	com.codenvy.ide.dashboard.CreateAndDeleteProjectsTest.createAndDeleteProjectTest
[TEST] 	com.codenvy.ide.filewatcher.RefactoringFeatureTest.checkRefactorFilesFromIde
[TEST] 	com.codenvy.ide.filewatcher.RemoveFilesWithActiveTabs.checkDeletionWithMultiOpenedTabFromIde
[TEST] 	com.codenvy.ide.filewatcher.RemoveFilesWithActiveTabs.checkDeletionWithSingleOpenedTabFromIde
[TEST] 	com.codenvy.ide.filewatcher.RemoveFilesWithActiveTabs.checkRemovingWithoutIde
[TEST] 	com.codenvy.ide.filewatcher.UpdateFilesWithoutIDE.checkEditingFileWithoutIDE
[TEST] 	com.codenvy.ide.gwt.CheckSdmModeGwtFeatureTest.checkLaunchingCodeserver
[TEST] 	com.codenvy.ide.projectexplorer.CheckErrorMessageWhenCreationDuplicateFolderOrFileTest.checkDuplicatedFolder
[TEST] 	com.codenvy.ide.projectexplorer.CheckOnValidAndInvalidClassNameTest.shouldNotCreateClassWithInvalidNameInJavaFolder
[TEST] 	com.codenvy.ide.projectexplorer.CheckOnValidAndInvalidClassNameTest.shouldNotCreateClassWithInvalidNameInRootPackage
[TEST] 	com.codenvy.ide.projectexplorer.CheckOnValidAndInvalidPackageNameTest.createInvalidPackageNameInJavaFolderTest
[TEST] 	com.codenvy.ide.projectexplorer.CheckOnValidAndInvalidPackageNameTest.createInvalidPackageNameInRootPackageTest
[TEST] 	com.codenvy.ide.subversion.DiffViewTest.checkDiffView
[TEST] 	com.codenvy.ide.swagger.SwaggerTest.checkNameProjectTest
[TEST]
[TEST] Comparing with https://ci.codenvycorp.com/view/qa/job/che-integration-tests/247/ (not the latest results)
[TEST] If a test failed then it is NOT marked as regression.
[TEST]

Rerun failed tests result:

[TEST]
[TEST] RESULTS ANALYSE:
[TEST]
[TEST] Command line: ./webdriver.sh --analyse 247
[TEST]
[TEST] CI results https://ci.codenvycorp.com/view/qa/job/che-integration-tests/247/
[TEST] 	- Failed:	81
[TEST]
[TEST] Local results:
[TEST] 	- Run:		34
[TEST] 	- Failed:	0
[TEST]
[TEST] Comparing with https://ci.codenvycorp.com/view/qa/job/che-integration-tests/247/ (not the latest results)
[TEST] If a test failed then it is NOT marked as regression.
[TEST]
[TEST] NO REGRESSION, great job!
[TEST]

@vinokurig vinokurig merged commit 7c68278 into master Mar 10, 2017
@vinokurig vinokurig deleted the CHE-3583 branch March 10, 2017 09:01
@vinokurig vinokurig added this to the 5.5.0 milestone Mar 10, 2017
@JamesDrummond JamesDrummond mentioned this pull request Mar 17, 2017
9 tasks
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants