CHE-2308. Use own size of part when part stack size is too small for the content of the part #2931

RomanNikitenko · 2016-10-28T12:58:59Z

What does this PR do?

We use default size for all part stacks now.
After applying my changes we will have next behavior:

for example you open Git History first, it uses 550 width. You open Pull Request panel while Git History was still displayed, it uses 550 width too.
you open Git History first, it uses 550 width. You close Git History and then open Pull Request panel while nothing was displayed on the right part, it uses default size (260) width.
you open Pull Request first, it uses 260 width. You open Git History while Pull Request was still displaying on the right, it uses 550 width.
you open Pull Request first, it uses 260 width. You close Pull Request and then open Git History panel while nothing was displayed on the right part, it uses 550 width.

This behavior will be apply for all part stacks.
We use default size (260) for all parts except Git history part (it will be 550).

What issues does this PR fix or reference?

#2308

Signed-off-by: Roman Nikitenko [email protected]

slemeur · 2016-10-28T13:07:52Z

Looks good for Git History.
Does that mean that it changed the default sizes for all panels displayed on the right?

I'm asking this because, obviously the default width for the Pull Request Panel and the Git History should not be the same - it should depend on the feature displayed.

RomanNikitenko · 2016-10-28T13:40:51Z

@slemeur
You are right, these changes apply for tooling part stack, so for Pull Request Panel as well as for the Git History.
As far as I remember we had issue some times ago to avoid jumps when we are switching between parts of the same part stack: between Events and Processes area for example. So I have applied this behavior for Pull request panel and Git History panel.
But I'm interested in your opinion - WDYT?

codenvy-ci · 2016-10-28T14:18:57Z

Build # 831 - FAILED

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

slemeur · 2016-10-28T14:44:56Z

I think that changing the sizes of the panel, once it has been opened would be ugly.

Imagine the following use case:
1- Open Pull Request panel --> small width for right part
2- Open Git History panel --> large width for right part
3- Open Pull Request panel --> small width for right part
It will show a lot of redrawing which would not really be friendly to the user.

There are probably a tradeoff possible, by increasing the width of the right part - only if the user try to display a panel which require it.

We would have the following use cases:
1- Open Pull Request panel --> small width for right part
2- Open Git History panel --> large width for right part
3- Open Pull Request panel --> width for right part stay large
And until the user has not opened git history we keep displaying it small.

This way we avoid constant redrawing and situation where the right part is too small for the content of the panel.

Does it make sense to you?

Maybe - for the time being it is overkill, but we can fill an improvement for that behavior.

RomanNikitenko · 2016-10-28T15:11:52Z

@slemeur
So do you prefer different default size for Git History panel and for Pull request panel?

Before my changes we had default size for both panel: 260
After my changes default size for both panel is: 750

Do you prefer follow default size:

for Git History panel - 750
for Pull request panel - 260

slemeur · 2016-10-28T15:38:53Z

Yeah, but I'd like the width being updated only if more space is needed.

So for example:

you open Git History first, it uses 750 width. You open Pull Request panel while Git History was still displayed, it uses 750 width too.
you open Git History first, it uses 750 width. You close Git History and then open Pull Request panel while nothing was displayed on the right part, it uses 260 width.
you open Pull Request first, it uses 260 width. You open Git History while Pull Request was still displaying on the right, it uses 750 width.
you open Pull Request first, it uses 260 width. You close Pull Request and then open Git History panel while nothing was displayed on the right part, it uses 750 width.

750 by default for all panel is too much.

RomanNikitenko · 2016-11-02T11:12:16Z

thank you, Stevan!
Now it more clear for me.
But I still have questions.
Current behavior also includes the following: we remember size when user resizes a part stack. For example:

user opens Processes panel with default size = 260
user resizes it to 550
user closes Processes panel

At this moment we remember 550 as size for bottom part stack.
So we will use 550 size when user opens Processes panel or Events panel as well.
I think we need to override this behavior for right part stack, otherwise corresponding the new behavior imagine the following use case:

user opens Git History first, it uses default size.
user resizes it to a half of screen size.
user opens Pull Request panel while Git History was still displayed - we remember this size as size for right part stack
Pull Request panel is displayed with a half of screen size.

What size we need to remember for right part stack?
Do we need to remember own size for each part (not for part stack)?
So do we need to have different way of handling left, right and bottom part stacks?

slemeur · 2016-11-03T08:53:18Z

Thanks Roman.

We need to find a solution which will be the same for each part stacks, not only for right part stack - otherwise it will be confusing for the user.

I think that it does not change the main goal that we want to achieve, we want to avoid situations where the user will see the size changing as he is switching between panels.

Let's keep the rule defined with the previous comments and override it once the user manually resized a part (all: left, right, bottom). If a user manually resized a part, we use it as default for all panels of the part.

codenvy-ci · 2016-11-29T18:29:46Z

Build # 1153 - FAILED

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

codenvy-ci · 2016-12-21T01:39:09Z

Build # 1395 - FAILED

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

TylerJewell · 2017-01-07T17:29:26Z

@vparfonov @RomanNikitenko - lets plan to update these files and get them merged in the next sprint. If you have 30 minutes this week, it would be nice to go ahead and do that now. Cleaning up old PRs.

…the content of the part Signed-off-by: Roman Nikitenko <[email protected]>

RomanNikitenko · 2017-01-19T13:53:36Z

@slemeur @vparfonov I have updated pull request - please review.

slemeur · 2017-01-19T14:07:00Z

@RomanNikitenko: Have you applied the latest comment I gave on this PR?

btw - you can use reviewers for the PR, it helps anybody to see who must validate the PR before merging it.

codenvy-ci · 2017-01-19T14:38:10Z

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1690/

RomanNikitenko · 2017-01-19T15:25:38Z

@slemeur

I have updated video which displays new behavior
thank you for your advice about reviewers - I will use it!

vitaliy-guliy · 2017-01-19T15:56:22Z

ide/che-core-ide-app/src/main/java/org/eclipse/che/ide/part/PartStackPresenter.java

+
+        if (activePart != null) {
+            updateWorkBenchPartSize();
+        }


Could you please add comments on new blocks you are added?
It will help developers to understand the logic of the code.

vitaliy-guliy · 2017-01-19T15:57:17Z

ide/che-core-ide-app/src/main/java/org/eclipse/che/ide/part/PartStackPresenter.java

+        }
+    }
+
+    private boolean isSizeOverridden(double size) {


Could you please add a little comments on methods?

codenvy-ci · 2017-01-19T22:28:19Z

Build # 1697 - FAILED

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

RomanNikitenko · 2017-03-01T16:00:38Z

Fixed by #3799

benoitf force-pushed the master branch from f0ee59a to aba9a87 Compare December 20, 2016 09:31

TylerJewell added sprint/next team/ide labels Jan 7, 2017

CHE-2308. Use own size of part when part stack size is too small for …

5a1ef1a

…the content of the part Signed-off-by: Roman Nikitenko <[email protected]>

RomanNikitenko force-pushed the CHE-2308 branch from bc8366f to 5a1ef1a Compare January 19, 2017 13:46

RomanNikitenko changed the title ~~CHE-2308. Change default size for tooling part stack~~ CHE-2308. Use own size of part when part stack size is too small for the content of the part Jan 19, 2017

RomanNikitenko requested review from vparfonov and slemeur January 19, 2017 15:22

vitaliy-guliy reviewed Jan 19, 2017

View reviewed changes

Add some comments

0bdc0c9

RomanNikitenko added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jan 20, 2017

vparfonov approved these changes Jan 24, 2017

View reviewed changes

RomanNikitenko closed this Mar 1, 2017

RomanNikitenko deleted the CHE-2308 branch March 1, 2017 16:02

benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Nov 2, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CHE-2308. Use own size of part when part stack size is too small for the content of the part #2931

CHE-2308. Use own size of part when part stack size is too small for the content of the part #2931

RomanNikitenko commented Oct 28, 2016 •

edited

Loading

slemeur commented Oct 28, 2016

RomanNikitenko commented Oct 28, 2016

codenvy-ci commented Oct 28, 2016

slemeur commented Oct 28, 2016 •

edited

Loading

RomanNikitenko commented Oct 28, 2016

slemeur commented Oct 28, 2016

RomanNikitenko commented Nov 2, 2016 •

edited

Loading

slemeur commented Nov 3, 2016

codenvy-ci commented Nov 29, 2016

codenvy-ci commented Dec 21, 2016

TylerJewell commented Jan 7, 2017

RomanNikitenko commented Jan 19, 2017

slemeur commented Jan 19, 2017

codenvy-ci commented Jan 19, 2017

RomanNikitenko commented Jan 19, 2017

vitaliy-guliy Jan 19, 2017

RomanNikitenko Jan 19, 2017

vitaliy-guliy Jan 19, 2017

RomanNikitenko Jan 19, 2017

codenvy-ci commented Jan 19, 2017

RomanNikitenko commented Mar 1, 2017

CHE-2308. Use own size of part when part stack size is too small for the content of the part #2931

CHE-2308. Use own size of part when part stack size is too small for the content of the part #2931

Conversation

RomanNikitenko commented Oct 28, 2016 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

slemeur commented Oct 28, 2016

RomanNikitenko commented Oct 28, 2016

codenvy-ci commented Oct 28, 2016

slemeur commented Oct 28, 2016 • edited Loading

RomanNikitenko commented Oct 28, 2016

slemeur commented Oct 28, 2016

RomanNikitenko commented Nov 2, 2016 • edited Loading

slemeur commented Nov 3, 2016

codenvy-ci commented Nov 29, 2016

codenvy-ci commented Dec 21, 2016

TylerJewell commented Jan 7, 2017

RomanNikitenko commented Jan 19, 2017

slemeur commented Jan 19, 2017

codenvy-ci commented Jan 19, 2017

RomanNikitenko commented Jan 19, 2017

vitaliy-guliy Jan 19, 2017

Choose a reason for hiding this comment

RomanNikitenko Jan 19, 2017

Choose a reason for hiding this comment

vitaliy-guliy Jan 19, 2017

Choose a reason for hiding this comment

RomanNikitenko Jan 19, 2017

Choose a reason for hiding this comment

codenvy-ci commented Jan 19, 2017

RomanNikitenko commented Mar 1, 2017

RomanNikitenko commented Oct 28, 2016 •

edited

Loading

slemeur commented Oct 28, 2016 •

edited

Loading

RomanNikitenko commented Nov 2, 2016 •

edited

Loading