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

[CLOSED] Added support for dragging the sidebar to resize it. #752

Open
core-ai-bot opened this issue Aug 29, 2021 · 9 comments
Open

[CLOSED] Added support for dragging the sidebar to resize it. #752

core-ai-bot opened this issue Aug 29, 2021 · 9 comments

Comments

@core-ai-bot
Copy link
Member

Issue by ryanstewart
Friday Apr 27, 2012 at 19:22 GMT
Originally opened as adobe/brackets#757


Made changes to support dragging the sidebar to resize it while also fixing some of the hide/show sidebar bugs.


ryanstewart included the following code: https://github.com/adobe/brackets/pull/757/commits

@core-ai-bot
Copy link
Member Author

Comment by ryanstewart
Friday Apr 27, 2012 at 19:28 GMT


I think this fixes #674 and also #659.

@core-ai-bot
Copy link
Member Author

Comment by ryanstewart
Friday Apr 27, 2012 at 22:39 GMT


Hmm, something's not working. Looking into it.

@core-ai-bot
Copy link
Member Author

Comment by ryanstewart
Friday Apr 27, 2012 at 23:15 GMT


Fixed a couple of bugs I found. This should be ready for review.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Saturday Apr 28, 2012 at 00:14 GMT


Drive-by feedback (not official in-depth review):

  • Double vs. single quotes inconsistent in initSidebarListeners() (I believe we generally use double quotes in all .js files)
  • Might be cleaner to move that long list of $(sidebar_resizer).css(...) calls into a CSS class
  • Stray console.log() on line 285
  • Hardcoding the selection triangle width (10?) feels fragile. Is there a way to retrieve that value by measuring a DOM node somewhere? If not, we should add a comment wherever that size is set in our LESS code warning that any changes to that number must be synced up with this JS code.
  • Should remove the commented-out line in the ".sidebar .scrollerShadow" rule (keep the comment that goes with it, though)
  • There's one more usage of@sidebar-width. Will it also get clobbered automatically? If so (seems like it'd have to be so), we should remove the width setting and add a similar comment there too.
  • Also, if we remove both usages of@sidebar-width we should remove the variable definition too (in brackets_variables.less)
  • Should remove the commented-out code in ViewUtils.js. May want to re-word comment accordingly.

@core-ai-bot
Copy link
Member Author

Comment by ryanstewart
Saturday Apr 28, 2012 at 20:55 GMT


Thanks Peter, went through and made those changes.@sidebar-width will still get used, I just commented it out for that shadow section. I didn't add the clobbered comment, it came from whoever was working on the project panel. Jason maybe?

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Tuesday May 01, 2012 at 22:49 GMT


Sure. I can look at project panel changes.

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Wednesday May 02, 2012 at 16:43 GMT


A few more notes:

  • The selectionMarker isn't resizing with the .sidebar
  • You have to kick codemirror using EditorManager.resizeEditor() to get the height correct. Watch the gap at the bottom of the window as you resize from a large .sidebar to smaller.

@core-ai-bot
Copy link
Member Author

Comment by ryanstewart
Wednesday May 02, 2012 at 17:21 GMT


Hmmm, after all these changes there are a couple of issues with the feature. It's definitely not as snappy and smooth as it was before. But I think there's a tradeoff there between smoothness and memory with what happens on the mousemove event.

And currently the selection triangle seems to jump around a lot which I think has something to do with the resizing. I'm thinking it might be better to close the request and start from scratch fixing some of the sidebar issues.

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Wednesday May 02, 2012 at 17:44 GMT


Agreed.

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

No branches or pull requests

1 participant