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.
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
Add site manager view and sidebar #1661
Add site manager view and sidebar #1661
Changes from 33 commits
4644ba0
742e68e
ed0b88a
4272269
3150b30
53967e4
1d1f3fb
7f2dbe7
cca77fd
09a4d16
f59de26
0c5b031
c937e00
58022b7
f2eed46
58e076b
b6c3ba3
cc9e90b
0ccc318
d69fe76
3206084
16b4875
8817d59
4f5700d
db154de
618afa7
5cb582a
3914f5b
87d5fff
ba559c5
4303ded
921df43
377a42e
0676c79
9448b3a
3b0c7c3
e3c7bb4
d9d1ed2
3728dd2
c23040b
9503c35
47d436f
84176fd
291c12a
edacf40
eff01f1
a8f3078
454ab61
463b2eb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
These transitions feel a bit jarring to me. Subjectively, they feel too fast and also out-of sync. Specifically, the side bar transition in and the toolbar transition out feel out-of-sync with one another and jerky.
I think one of the reasons is timing, and the other may be that the sidebar appears immediately on the left while the contents slide into the sidebar from the right.
Screen.Recording.2024-08-06.at.2.54.19.PM.mov
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.
You are right, this isn't great. I updated the animation to work similarly to the
site-editor
and synced the sidebar and toolbar animations. How does this look?Screen.Recording.2024-08-07.at.11.19.39.mov
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.
The back animation could be a bit smoother, but I couldn't find a good way of doing it because the sidebar component is removed from DOM.
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.
This looks much better to me, and I no longer feel jarred when opening the sidebar.
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.
Why is the toolbar hidden when the sidebar is expanded? As a user, it feels like a distracting change and one that unnecessarily takes away the embedded WP navigation bar.
It seems practical to take away (or disable) the site configuration menus while the sidebar is opened to avoid conflicts between the two, but would it be possible to slide those out and let the embedded nav bar remain so users can continue seeing and setting the WP URL for the active session?
Leaving the toolbar in place might also reduce the number of moving parts mentioned in the "jarring" feedback above.
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 wanted it to match the future design where the toolbar won't be visible.
Also the idea was for the site view to be disabled here but I changed it based on @adamziel feedback.
The current toolbar will eventually be removed, so I don't want to spend time adjusting it to work here for example we would need to hide the new menu button, adjust it to match visually, and add extra breakpoints to keep responsive support.
@jarekmorawski what do you think?