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.

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

fix: User avatar shrinks on mobile when action is submitting #2559

Merged
merged 2 commits into from
Jun 21, 2024

Conversation

Nortsova
Copy link
Contributor

@Nortsova Nortsova commented Jun 18, 2024

Description

Fix user avatar shrinks on mobile when action is submitting

Testing

  • Step 1. Set screen size to mobile
  • Step 2. Create a new action and submit
  • Step 3. See that user avatar is visible
image

Resolves #2546

@Nortsova Nortsova requested review from a team as code owners June 18, 2024 11:04
@CLAassistant
Copy link

CLAassistant commented Jun 18, 2024

CLA assistant check
All committers have signed the CLA.

davecreaser
davecreaser previously approved these changes Jun 18, 2024
Copy link
Contributor

@davecreaser davecreaser left a comment

Choose a reason for hiding this comment

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

Good stuff, all tested and the avatar stays visible!

Screenshot 2024-06-18 at 16 25 15

@rumzledz rumzledz self-requested a review June 18, 2024 16:02
rumzledz
rumzledz previously approved these changes Jun 18, 2024
Copy link
Contributor

@rumzledz rumzledz left a comment

Choose a reason for hiding this comment

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

Looking brill @Nortsova

Screenshot 2024-06-18 at 17 01 55

Copy link
Contributor

@iamsamgibbs iamsamgibbs left a comment

Choose a reason for hiding this comment

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

Congratulations on your first PR @Nortsova 🎉

The fix does indeed solve the issue:

Screen.Recording.2024-06-18.at.17.12.35.mov

However, I think this change is unnecessary and is affecting a different part of the header:

Screenshot 2024-06-18 at 17 13 11

And flex is already included in the button component and doesn't need to be included again here:

Screenshot 2024-06-18 at 17 11 47

Nice job identifying the components involved and making the fix though! I'll review again after these changes.

@@ -151,7 +151,7 @@ const UserHubButton: FC = () => {
{
'!border-blue-400': visible || isUserHubOpen,
},
'md:hover:!border-blue-400',
' flex min-w-[3rem] md:hover:!border-blue-400',
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm being incredibly nitpicky, but there is an unnecessary space at the start of this line.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also don't think flex is needed here as it is already included in the button component.

@@ -117,7 +117,7 @@ const NavigationSidebarContent: FC<NavigationSidebarProps> = ({
md:gap-9
`}
>
<div className="flex gap-6 md:flex-col md:gap-9">
<div className="flex gap-3 md:flex-col md:gap-9">
Copy link
Contributor

Choose a reason for hiding this comment

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

This change affects a different part of the header.

@Nortsova Nortsova dismissed stale reviews from rumzledz and davecreaser via f0c6171 June 18, 2024 16:29
Copy link
Contributor

@iamsamgibbs iamsamgibbs left a comment

Choose a reason for hiding this comment

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

Looks good to me now! Thanks for making the changes :)

Screenshot 2024-06-19 at 10 15 46

@Nortsova Nortsova requested review from davecreaser and rumzledz June 19, 2024 09:49
Copy link
Member

@rdig rdig left a comment

Choose a reason for hiding this comment

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

Can confirm user avatar no longer shrinks when creating a new action in mobile view

Screenshot from 2024-06-19 21-14-31

@mmioana mmioana self-requested a review June 21, 2024 08:21
Copy link
Contributor

@mmioana mmioana left a comment

Choose a reason for hiding this comment

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

Avatar no longer shrinks

Screenshot 2024-06-21 at 10 25 23

Great work @Nortsova! 🥳

@Nortsova Nortsova merged commit 6bfddbb into master Jun 21, 2024
3 of 6 checks passed
@Nortsova Nortsova deleted the fix/2546-user-avatar-mobile branch June 21, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User avatar shrinks on mobile when action is submitting
7 participants