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: #1348 Fixed the follow button movement UI #1383

Merged
merged 5 commits into from
Jan 23, 2023

Conversation

rkmdCodes
Copy link
Contributor

hi I am a new contributor and I have fixed the movement in follow button UI

Here is the Gif attached ...
gif

@stackblitz
Copy link

stackblitz bot commented Jan 22, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@netlify
Copy link

netlify bot commented Jan 22, 2023

Deploy Preview for elk-docs canceled.

Name Link
🔨 Latest commit bff1823
🔍 Latest deploy log https://app.netlify.com/sites/elk-docs/deploys/63ce698d7077130008b8d973

@netlify
Copy link

netlify bot commented Jan 22, 2023

Deploy Preview for elk-zone ready!

Name Link
🔨 Latest commit bff1823
🔍 Latest deploy log https://app.netlify.com/sites/elk-zone/deploys/63ce698d4aa935000903ec82
😎 Deploy Preview https://deploy-preview-1383--elk-zone.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@rkmdCodes rkmdCodes changed the title fix : #1353 , Fixed the follow button UI fix:#1353,Fixed the follow button UI Jan 22, 2023
@rkmdCodes rkmdCodes changed the title fix:#1353,Fixed the follow button UI fix: #1348 Fixed the follow button movement UI Jan 22, 2023
@@ -103,7 +103,7 @@ const isNotifiedOnPost = $computed(() => !!relationship?.notifying)
<AccountHandle :account="account" />
</div>
</div>
<div absolute top-18 inset-ie-0 flex gap-2 items-center>
<div absolute top-18 inset-ie-0 flex gap-2 justify-center>
Copy link
Member

Choose a reason for hiding this comment

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

items-center here is correct. The issue is that the bell notification button is taller than the follow button. If the bell notification button size is reduced to match the follow button, then there will not be a pixel shift

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review!
I edited the commit and reverted the previous changes justify-center back to item-center
I found out that the issue is resolved by changing the padding of bell button from 0.5 rem to 0.4 rem p2 to p1.6 but I am getting type check error in CI check. I am still working on the issue.

Copy link
Member

Choose a reason for hiding this comment

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

I think you needed to add class="p1.6" for this to work. I changed it back to p2 though and made the icon text-sm so we have more coherent padding.

rkmdCodes and others added 3 commits January 23, 2023 13:33
Edited the padding of the bell notification button to match with follow button
Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

Thanks!

@patak-dev patak-dev enabled auto-merge (squash) January 23, 2023 11:04
@patak-dev patak-dev merged commit 8b72e17 into elk-zone:main Jan 23, 2023
@rkmdCodes
Copy link
Contributor Author

I learned a lot and this was my first Open source project!

@patak-dev
Copy link
Member

@rkmdCodes it gets better and better! Welcome to Elk and OSS 🙌🏼

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.

2 participants