-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Try adding a resize handle to resize the canvas #46466
Conversation
Size Change: +457 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
Damn, fast work. This resizing of the viewport is sweet: A few quick notes: for handles, I think we should have them on both sides, left and right. Here's an older mockup by Jay: The handles are $gray-600 in that mockup, 4px wide and 100px tall, and 8px from the edge. Ideally we can center the viewport when scaled down, that would make the dragging behavior more natural, presumably: On a separate note, there's something weird going on with the sidebar in this one. Trunk looks like this: But in this PR I see a dark back button and smaller font size: |
I'm not seeing the styling issues you're seeing, also for me the frame is centered when you resize it's not stuck on the left |
Oh right! should be fixed now :) |
Oh nice, that helps: Probably want a max-width applied to the thing, so you can't resize it infinitely. Also, even though the resize handle is 4px wide, I wonder if we could make its effective hit-area bigger? If it's 4px wide and has 8px margin left and right, theoretically we could have it be 20px wide. Not something to block the effort, but would be nice. |
e2a5def
to
f42fefe
Compare
Probably a min-width too: 360px or so? The resize handle momentarily overlaps the frame when moving from edit mode to browse mode which looks a bit awkward. I suppose ideally it would be positioned relative to the frame boundary and animate with it. But if that's not possible then adjusting the Perhaps something to do separately, but it might be reasonable for the sidebar to grow relatively as the frame shrinks. That way the feature doubles as a general UI preference (a wider sidebar might be desirable for management tasks) in addition to device previewing. |
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.
From the code point of view looks good and I don't see any blocker 👍
Gave this a quick try, it should be possible but it's a bit impactful as it would force us to rework the "layout" entirely. Here are two important aspects this impacts:
For these reasons, it's better done separately. |
5447eed
to
340d379
Compare
This one is close, but two things. First off: shouldn't there be two resize handles? One on each side? And the other thing, I noticed that the box-shadow on the container lags, I recorded this video to show it: Screen.Recording.2022-12-20.at.10.05.57.movNotice how as I resize, the shadow lags along. This appears related to the animation that resizes the viewport. IMO I don't think we need animation when you're resizing, it feels most snappy when it's instant, and directly attached to your mouse movements. Is it possible to turn off the animation here? See also #46354 |
you're right but I think we need to make a decision here:
You're right that the current behavior is close to 1, so we should probably add another handle now. |
Oh wait that's right. One idea is to allow you to resize the left handle leftwards to minimize the left navigation and "snap into fullscreen", as someting to explore, sorry I forgot that. One of the reasons that doesn't feel intuitive with the current implementation is that the browse view stays centered. I.e. when resizing the left handle, the right side of the viewport is also moved. Not only that, but the left menu does not grow wider or thinner. Which is to say, behavior 2 as you are describing, probably should be a bit more like a classic 2-frame setup (legacy frameset here): If we want that, we should move the behavior towards that by keeping the right edge of the viewport fixed to the right side. Right? CC: @jameskoster in case I'm missing something here. |
@jasmussen yes, but I'm explaining here that doing that means some big structural changes that I'd prefer to delay for a separate PR #46466 (comment) |
I've added the right resize handle. So either:
I'm fine with either approach :) |
I lean towards two handles, keeping the centering, etc. And I would still think that if we want the ability to "scale up" and snap to fullscreen, we can do that. But only with the left handle. You grab it, and drag leftwards, and at that point, the right handle stays put. What do you think? |
Not sure how feasible yet with the current structure as the left border is kind of "stuck" prevent us from going further (at least visually) |
We can probably cross the bridge when we get there. |
Is it worth spend a little time exploring the design here? I think resizing the UI (IE changing the width of the left panel) will be useful, and it's something that would likely persist outside of the editor. Whereas resizing the preview is an editor-only feature, and might be better served by icon buttons with preset values. I suppose the question is – should these resize tools be separate? Quick mockup: |
Since you bring it up, I think there's a question of whether we want to see the resize handles all the time or not. Visually I like their appearance, and that they share DNA with the focus mode. But most of the time you might not want to resize the browse view. I wonder if they could appear when hovering either the space left and right of the viewport? |
Yes the handles feel a bit too prominent currently. That's actually what lead me to the icon button presets, assuming that mot folks don't know device breakpoints by heart I wondered if they might be more practical in this context? |
Just a quick note that this was requested as part of the FSE Outreach Program's nineteenth call for testing that partially explores browse mode (and more):
|
Going to close this PR and create a new one based on the new layout in trunk. |
Alternative PR opened here #46903 |
Related #36667
What?
This PR adds the ability to resize the canvas size. It also removes the resizing that was specific to the template parts as it's a bit redundant.
Here's what it looks like so far
Screen.Recording.2022-12-19.at.9.52.17.AM.mov
Notes:
Testing instructions