Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(UIShell): update story to show proper right panel transition #9364

Merged
merged 4 commits into from
Aug 3, 2021
Merged

fix(UIShell): update story to show proper right panel transition #9364

merged 4 commits into from
Aug 3, 2021

Conversation

tw15egan
Copy link
Collaborator

Closes #9095

Updates the UI Shell with Actions and Right Panel to properly use state to manage the expansion so that it shows the proper transitions.

Changelog

Changed

  • Updates the story to show proper animations

Testing / Reviewing

Go to the Header Base w/ Actions and Right Panel with Animation story and click the notification icon. Ensure the transition works as expected

@tw15egan tw15egan requested a review from a team as a code owner July 29, 2021 17:55
@tw15egan tw15egan requested review from sstrubberg and jnm2377 July 29, 2021 17:55
@tw15egan tw15egan requested a review from laurenmrice July 29, 2021 17:56
@netlify
Copy link

netlify bot commented Jul 29, 2021 •
edited
Loading

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: e78c734

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/6109639a5e6bfe0007ed9eaa

😎 Browse the preview: https://deploy-preview-9364--carbon-react-next.netlify.app

@netlify
Copy link

netlify bot commented Jul 29, 2021 •
edited
Loading

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: e78c734

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/6109639a2670840007fa6201

😎 Browse the preview: https://deploy-preview-9364--carbon-components-react.netlify.app

@netlify
Copy link

netlify bot commented Jul 29, 2021 •
edited
Loading

❌ Deploy Preview for carbon-elements failed.

🔨 Explore the source changes: e78c734

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/6109639a4f03720007600320

sstrubberg
sstrubberg approved these changes Jul 30, 2021
View reviewed changes
Copy link
Member

@sstrubberg sstrubberg left a comment

Choose a reason for hiding this comment

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

Jul-30-2021 11-01-42

🚀

laurenmrice
laurenmrice reviewed Aug 2, 2021
View reviewed changes
Copy link
Member

@laurenmrice laurenmrice left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • Just wondering if the open panel should also be connected to the header icon it was triggered from.

Ex:
Screen Shot 2021-08-02 at 1 24 32 PM

  • This tooltip gets cutoff and should probably have the caret tip positioned on the right side, so the tooltip opens to the left.

Screen Shot 2021-08-02 at 1 28 11 PM

@tw15egan tw15egan force-pushed the update-ui-shell-story branch from 7bdeb07 to 8ecb57a Compare August 2, 2021 18:41
@tw15egan
Copy link
Collaborator Author

tw15egan commented Aug 2, 2021

@laurenmrice fixed, nice catch 👍🏻

laurenmrice
laurenmrice requested changes Aug 3, 2021
View reviewed changes
Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

  • The tooltip position and icon to panel connection look good!

  • One last thing I noticed is that when you click the icon in the header to open the panel, it shifts down a little bit in the header. So then it looks like its jumping around when you close or open the panel. This happens in Firefox/Chrome/Safari. My giph is taken from Safari as an example:
    Aug-03-2021 10-41-22

@tw15egan tw15egan force-pushed the update-ui-shell-story branch from 8ecb57a to 72511db Compare August 3, 2021 14:54
@tw15egan
Copy link
Collaborator Author

tw15egan commented Aug 3, 2021

@laurenmrice should be fixed now, thanks for catching it!

laurenmrice
laurenmrice approved these changes Aug 3, 2021
View reviewed changes
Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

Looks great! 🎉

@kodiakhq kodiakhq bot merged commit 6d4c76e into carbon-design-system:main Aug 3, 2021
This was referenced Aug 3, 2021
Merged
Merged
@tw15egan tw15egan deleted the update-ui-shell-story branch March 3, 2022 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurenmrice laurenmrice laurenmrice approved these changes

@jnm2377 jnm2377 jnm2377 approved these changes

@sstrubberg sstrubberg sstrubberg approved these changes

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Transition in right sidebar not working
4 participants
@tw15egan @sstrubberg @jnm2377 @laurenmrice