Skip to content

Snap Sidebar To Width - #882

Merged
austincondiff merged 5 commits into
CodeEditApp:mainfrom
yapryntsev:feature/yapryntsev/#856/sidebar-width-snap
Jan 3, 2023
Merged

Snap Sidebar To Width#882
austincondiff merged 5 commits into
CodeEditApp:mainfrom
yapryntsev:feature/yapryntsev/#856/sidebar-width-snap

Conversation

@yapryntsev

@yapryntsev yapryntsev commented Jan 1, 2023

Copy link
Copy Markdown
Contributor

Description

  • Added CodeEditSplitViewController which handles all snap logic.
  • NavigatorSidebarToolbarTop wrapped in GeometryReader. Now, HStack can react to width changes and choose appropriate space between items.
  • Add unit tests for CodeEditSplitViewController.

Related Issue

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • I documented my code
  • Review requested

Screenshots

photo_2023-01-01_15-06-23

@yapryntsev yapryntsev changed the title Feature/yapryntsev/#856/sidebar width snap Snap Sidebar To Width Jan 1, 2023
@austincondiff

austincondiff commented Jan 1, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Great work @yapryntsev! Would you mind posting a short screen video of resizing the sidebar? Also, could you fix the proportions of the symbols in the sidebar dock?

lukepistrol
lukepistrol previously approved these changes Jan 2, 2023
View reviewed changes

@lukepistrol lukepistrol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

0xWDG
0xWDG previously approved these changes Jan 2, 2023
View reviewed changes

@0xWDG 0xWDG left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Please note Austins comments.

@yapryntsev

yapryntsev commented Jan 3, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@austincondiff, no problem, I'll update PR later tonight

@yapryntsev
yapryntsev dismissed stale reviews from 0xWDG and lukepistrol via d20e08f January 3, 2023 11:34
@yapryntsev

yapryntsev commented Jan 3, 2023

Copy link
Copy Markdown
Contributor Author
IMG_4530.MOV

@yapryntsev
yapryntsev requested review from 0xWDG and lukepistrol and removed request for 0xWDG and lukepistrol January 3, 2023 11:42
lukepistrol
lukepistrol requested changes Jan 3, 2023
View reviewed changes
Comment thread CodeEdit/Features/NavigatorSidebar/NavigatorSidebarToolbarTop.swift
lukepistrol
lukepistrol approved these changes Jan 3, 2023
View reviewed changes

@lukepistrol lukepistrol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

austincondiff
austincondiff approved these changes Jan 3, 2023
View reviewed changes

@austincondiff austincondiff left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work, way to go.

@austincondiff
austincondiff merged commit 5e6cebe into CodeEditApp:main Jan 3, 2023
@austincondiff

austincondiff commented Jan 3, 2023

Copy link
Copy Markdown
Collaborator

@allcontributors add @yapryntsev for code

@allcontributors allcontributors Bot mentioned this pull request Jan 3, 2023
Merged
@allcontributors

allcontributors Bot commented Jan 3, 2023

Copy link
Copy Markdown
Contributor

@austincondiff

I've put up a pull request to add @yapryntsev! 🎉

@austincondiff austincondiff mentioned this pull request Jan 10, 2023
Open
11 of 23 tasks
@lukepistrol

lukepistrol commented Jan 11, 2023

Copy link
Copy Markdown
Member

@yapryntsev It seems your tests are very inconsistent as they regularly but randomly fail.

Please test this out yourself by running the tests repeatedly for 100 times and create a follow-up PR referencing this one.

These are the tests in question:

Screenshot 2023-01-11 at 03 37 58

When running it for 100 times, it will fail about 5-10 times.

@lukepistrol lukepistrol mentioned this pull request Jan 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@austincondiff austincondiff austincondiff approved these changes
@lukepistrol lukepistrol lukepistrol approved these changes
@0xWDG 0xWDG Awaiting requested review from 0xWDG
@matthijseikelenboom matthijseikelenboom Awaiting requested review from matthijseikelenboom
@thecoolwinter thecoolwinter Awaiting requested review from thecoolwinter

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yapryntsev @austincondiff @lukepistrol @0xWDG