Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Add a maxsize check for filetree resize… #13026

Merged
merged 1 commit into from
Jan 17, 2017
Merged

Conversation

ficristo
Copy link
Collaborator

@ficristo ficristo commented Jan 7, 2017

… and now it is possible to pass a percentage value.
It replaces #9122 and #12829
This fixes #6822, I'm sorry to take a starter bug but I thought it was open long enough that's time to fix it.

@redmunds could you review?

@ficristo
Copy link
Collaborator Author

ficristo commented Jan 7, 2017

Argh... I noticed only now #10456. @redmunds WDYT?

@redmunds
Copy link
Contributor

redmunds commented Jan 8, 2017

@ficristo I'm surprised this issue wasn't fixed long ago. Thanks for putting up a PR.

Your code works as advertised. LGTM.

@lucaslouca Brings up a good point in PR #10456:

Also important is the scenario when the Sidebar is at its maximum size (filling the whole window) and the user decreases the Window's width.

You can hit this case even when sidebar is less than 100%. I was going to try out this code, but the remote branch seems to have been deleted, so I'm not sure if it actually fixes that case.

Do you want to try to fix that in this PR? If not, this is worth merging anyway.

@ficristo
Copy link
Collaborator Author

ficristo commented Jan 9, 2017

With the new commit, it should take care of the window resize.
I tryed to use maxsize= 100% but it has the old problem but I think it is fine for now.

@redmunds
Copy link
Contributor

@ficristo Nice. Works great. I have a minor suggestion, but feel free to merge PR as is.

When window size is reduced, you enforce that leftSidebar.width is <= window.width - rightSidebar.width. Why don't you also apply the 80% to that calculation? This will maintain the same behavior as when dragging leftSidebar with.

@ficristo
Copy link
Collaborator Author

Now it should take care of the percentage.
But I noticed an issue with the selection of current file...
current-file

This happen if you:

  • resize the filetree at max
  • resize the window to go over the file tree
  • make sure the filetree was resized automatically
  • resize the window to enlarge it

@redmunds
Copy link
Contributor

That only happens if there are enough files in WorkingSet to require scrollbars. That line is the shadow at bottom of WorkingSetView. The editor view is not getting refreshed.

@ficristo
Copy link
Collaborator Author

Triggering the panelResizeUpdate and panelResizeEnd events seems to have fixed it.
Triggering only one had other side effects.
It feels a bit dirty but I don't have other ideas at the moment.

@redmunds
Copy link
Contributor

@ficristo I think triggering those events is the correct fix, although panelResizeStart should also be called to satisfy the API. Also, 1 typo, then LGTM.

@ficristo
Copy link
Collaborator Author

I've added the call to panelResizeStart event and rebased.
Which is the typo?

return !$.isNumeric(value) && value.indexOf('%') > -1;
}

function _percetageToPixels(value, total) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Your typo is percetageToPixels -> percentageToPixels. Also in all the function calls.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for noticing! Fixed.

@redmunds
Copy link
Contributor

LGTM

@redmunds redmunds merged commit be471e8 into adobe:master Jan 17, 2017
@ficristo
Copy link
Collaborator Author

@redmunds @haslam22 Thank you for the review.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sidebar can't be resized once filling whole window
3 participants