Skip to content

Conversation

winston-de
Copy link
Contributor

@winston-de winston-de commented May 24, 2021

Resolved / Related Issues
Items resolved / related issues by this PR.

Details of Changes
Add details of changes here.

  • Added ability to close and open sidebar by resizing it
  • Added constant for minimum sidebar size
  • Added visual effects for drag and hover over sidebar resizer
  • Fixed only being able to drag sidebar by border

Validation
How did you test these changes?

  • Built and ran the app
  • Tested the changes for accessibility

Screenshots (optional)

2021-05-23-185548.mp4

@winston-de winston-de marked this pull request as draft May 24, 2021 01:19
@winston-de winston-de marked this pull request as ready for review May 24, 2021 02:17
@yaira2 yaira2 self-requested a review May 24, 2021 02:24
@yaira2 yaira2 added the ready for review Pull requests that are ready for review label May 24, 2021
@yaira2
Copy link
Member

yaira2 commented May 24, 2021

@winston-de Is the resize janky on your end?

@winston-de
Copy link
Contributor Author

Yeah a bit, I thought it may have just been my computer. I'll try and improve it.

@mdtauk
Copy link
Contributor

mdtauk commented May 24, 2021

Not sure how possible it is, but it feels like the sidebar resizing should snap to the closed state, when dragged below a minimum width.

If there is also a maximum width, the same should probably apply after the threshold is reached, when resizing outward.

@winston-de
Copy link
Contributor Author

Not sure how possible it is, but it feels like the sidebar resizing should snap to the closed state, when dragged below a minimum width.

That's what it does now, unless I'm misunderstanding what you're saying.

If there is also a maximum width, the same should probably apply after the threshold is reached, when resizing outward.

I don't quite understand what you mean by this....

@winston-de
Copy link
Contributor Author

@yaichenbaum It looks like the jankiness was already there before this PR. I think it's caused by binding directly to the SidebarWidth app setting directly and updating it on manipulation delta, leading to a lot of calls to get and set app settings. I'm working on improving it though.

@yaira2
Copy link
Member

yaira2 commented May 24, 2021

@yaichenbaum It looks like the jankiness was already there before this PR. I think it's caused by binding directly to the SidebarWidth app setting directly and updating it on manipulation delta, leading to a lot of calls to get and set app settings. I'm working on improving it though.

Interesting, it must have been introduced with all the recent refactoring around the sidebar, I'm glad we found it now!

@yaira2
Copy link
Member

yaira2 commented Jun 14, 2021

@winston-de are you still working on this?

@winston-de
Copy link
Contributor Author

winston-de commented Jun 14, 2021

@winston-de are you still working on this?

Yes, I'll have it done in a bit.

@winston-de
Copy link
Contributor Author

Alright I think I fixed the jankiness.

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed ready for review Pull requests that are ready for review labels Jun 14, 2021
@yaira2
Copy link
Member

yaira2 commented Jun 14, 2021

@winston-de Can you resolve the merge conflicts and test these changes with the keyboard?

@yaira2 yaira2 merged commit de694dd into main Jun 15, 2021
@yaira2 yaira2 deleted the ResizeToCollapseSidebar branch June 15, 2021 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge Pull requests that are approved and ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ability to close sidebar by dragging

3 participants