This repository has been archived by the owner on Sep 6, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Sidebar resize refactorization #1811
Closed
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
6d4e21c
Initial sidebar resizing
jbalsas b1cb8f2
Resizes shadow and triangle selection
jbalsas 12b399c
Add end handler to save width preference
jbalsas 8c0c38b
Initial panel toggle
jbalsas f88be8f
Add support for toggling an element
jbalsas b672c26
Adds support for snap resize
jbalsas 9169b27
Fix sidebar handler on codemirror gutter space
jbalsas 0c986bb
Comments and cleanup
jbalsas 57766a7
Removed unused EditorManager and isBarClosed
jbalsas 6bff0ca
Merge branch 'master' of https://github.com/adobe/brackets into sideb…
jbalsas cbe2209
Fix size glitch when starting resize from hidden panel
jbalsas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be much better if we didn't need to update this rule every time someone adds a new panel. This should only be the "resizing" and "horz-resizing" classes. Maybe we need to adjust where those classes are applied, but I think it's worth the change.
Same comment for next rule with "vert-resizing".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look into it before creating the pull request again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@redmunds It's okay if we do
$("*").toggleClass("vert-resizing")
on theResizer
module? Or could that create some hit on performance?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That will remove the "vert-resizing" class from the few elements that have it, and add it to every other element in the DOM! I don't think that's what you want to do.
You can use $(".vert-resizing").toggleClass("vert-resizing") to remove the "vert-resizing" class from all elements that have it applied. Note: after that you won't be able to identify the elements that used to have it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming that's what you want, this is a little better: $(".vert-resizing").removeClass("vert-resizing")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes... I was actually thinking of doing
$("*").addClass("vert-resizing")
on resize start and then the removeClass on the resize end. This way, the handle cursor isn't overridden by other rules, and we can take the resizing classes outside and shouldn't be affected by any element changing the cursor style...I assume that's what you meant in the beginning.. or maybe I'm missing the point?
If this is not entirely satisfactory, maybe we can leave it for now and think it through.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you're saying makes sense, but won't $("*") return every element in the entire Brackets DOM tree? I think you want to limit it to the panel that's being resized. We can talk about it more when you push up a pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's why I wanted to avoid it. I expect $("*") to be really slow.
I think I've found a nice solution that could work creating a transparent div just for the resizing operation. I'll put together a pull request and then we can discuss it there.
Thanks for the support!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@redmunds See #1820