Bug #6822. Provided maxSize property for the sidebar and added a check in resizer.js to convert maxSize into pixels if provided in percentage.#9122
Conversation
There was a problem hiding this comment.
There needs to be a space between if and ( as well as between ) and { (see if block on line 339 just below for an example).
Add the proper indentation as per the coding standards
|
Changes done as per comment |
There was a problem hiding this comment.
Better to do this in sidebarview.js
You will also need to update the size when the window size changes plus if it's smaller than the max size after resizing you will need to resize the sidebar to the new max size. Resizer doesn't do this for you.
There was a problem hiding this comment.
An even cleaner approach would be:
- Add
data-maxsize="50%"to the sidebar DOM (it's in main-view.html) - When fetching maxSize here, check if it's a string ending in "%" and dynamically convert it to a number in pixels
This way there's no hardcoded knowledge about the sidebar in the generic Resizer code, and there are also no special resize listeners needed in the sidebar code.
(This is basically what I suggested in #7670 (comment) too).
|
Made the changes, this was a better approach indeed. |
There was a problem hiding this comment.
Why are you adding a blank line here, where there was none before?
|
@redmunds do you know if this is still relevant? |
|
@ficristo Issue #6822 is still open, so I think this is still relevant. The contributor seems to have addressed all comments (although didn't add a "ready for another review" comment), but I have not verified the fix. One change I would like to see is to have the max be larger (something like 80%). It might not seem useful to only show 20% of window for editor, but you might want to temporarily expand to find a particular file. |
| <!-- Main UI: horizontal set of sidebar, main content vertical stack, and vertical toolbar --> | ||
| <div class="main-view"> | ||
| <div id="sidebar" class="sidebar panel quiet-scrollbars horz-resizable right-resizer collapsible" data-minsize="0" data-forceleft=".content"> | ||
| <div id="sidebar" class="sidebar panel quiet-scrollbars horz-resizable right-resizer collapsible" data-minsize="0" data-maxsize="45%" data-forceleft=".content"> |
There was a problem hiding this comment.
I think max should be large. Maybe 80% ?
|
Thank you @redmunds |
|
Thank you for contributing to Brackets. |
Provide maxSize to be 45% of window width if element is side bar and maxSize is not specified
This is for #6822.