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-7040: Add 400 ms. delay in Navigate to file dialog #8177

Merged
merged 7 commits into from
Jan 10, 2018
Merged

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Jan 4, 2018

What does this PR do?

Add 400 ms. delay in the Navigate to file dialog. This prevents sending request to server if the file name field was updated during the timeout. Timeout in 500 ms. was chosen because It is enough to type a new symbol when typing a file name, and on the other hand this is not a big delay between end of typing and sending a request.

What issues does this PR fix or reference?

#7040

Prevent sending search request to server from Navigate to file dialog, if the file name field was updated.

Release Notes

N/A

Docs PR

N/A

@vinokurig vinokurig added kind/enhancement A feature request - must adhere to the feature request template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Jan 4, 2018
@vinokurig vinokurig requested a review from tolusha January 4, 2018 13:27
@vinokurig vinokurig requested a review from mmorhun January 4, 2018 13:28
() -> Log.error(getClass(), "Project search request failed due timeout"));
}
};
timer.schedule(500);
Copy link
Contributor

Choose a reason for hiding this comment

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

Move to constant.

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

@vinokurig vinokurig added status/in-progress This issue has been taken by an engineer and is under active development. and removed status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Jan 4, 2018
@vinokurig
Copy link
Contributor Author

ci-test

@vinokurig vinokurig changed the title CHE-7040: Add 500 ms. delay in Navigate to file dialog CHE-7040: Add 400 ms. delay in Navigate to file dialog Jan 5, 2018
@codenvy-ci
Copy link

codenvy-ci commented Jan 5, 2018

ci-test build report:
Build details
Test report
selenium tests report data
docker image: riuvshin/che-server:8177
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@@ -39,12 +39,16 @@
@Singleton
public class NavigateToFilePresenter implements NavigateToFileView.ActionDelegate {

private static final int TYPING_PERIOD_DELAY_MS = 400;
Copy link
Contributor

Choose a reason for hiding this comment

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

To me TYPING_REQUEST_DELAY_MS is clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not a request

Copy link
Contributor

Choose a reason for hiding this comment

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

query?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not a query. The idea is to identify that typing is not finished yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see

}

private void prepareResults(ProjectSearchResponseDto response) {
List<SearchResultDto> results = response.getItemReferences();
sort(
results,
results.sort(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be better to create static comparator?

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

delegate.onFileNameChanged(fileName.getText());
String fileName = NavigateToFileViewImpl.this.fileName.getText();
// Skip handling non letter characters.
if (nativeKeyCode == KEY_BACKSPACE
Copy link
Contributor

Choose a reason for hiding this comment

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

We can check it before timer creation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vinokurig
Copy link
Contributor Author

ci-test

@vinokurig vinokurig added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. and removed status/in-progress This issue has been taken by an engineer and is under active development. labels Jan 9, 2018
@codenvy-ci
Copy link

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:8177
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@vinokurig vinokurig merged commit 2365e30 into master Jan 10, 2018
@vinokurig vinokurig deleted the CHE-7040 branch January 10, 2018 07:15
@vinokurig vinokurig removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jan 10, 2018
@vinokurig vinokurig added this to the 6.0.0-M5 milestone Jan 10, 2018
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.

4 participants