-
-
Notifications
You must be signed in to change notification settings - Fork 77
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: update subscription component name and styles #344
Conversation
The subscription component name was misspelled as "Subcription". This commit fixes the typo and updates the component styles to improve the layout on different screen sizes.
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.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
✅ Deploy Preview for peaceful-ramanujan-288045 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
LGTM!
@ALIPHATICHYD please mention the issue in your pr description |
Just did that now |
components/Form/subscription.js
Outdated
type='submit' | ||
className='w-full md:w-[200px] mt-8 px-10' | ||
> | ||
Subscribe | ||
</Button> |
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.
@ALIPHATICHYD @ashmit-coder why so much whitespace though? 🤔
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'm so sorry about that, I'd make sure I adjust that
I didn't know it will come out that way.
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.
True, please look into it.
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'm so sorry about that, I'd make sure I adjust that I didn't know it will come out that way.
No problem. It happens sometimes due to auto-formatting extensions running wild. Thanks for the PR @ALIPHATICHYD
@ashmit-coder, @AnimeshKumar923 is this pr up for merge? |
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.
LGTM! ✅
cc: @AceTheCreator
Just a small nitpick about your PR description @ALIPHATICHYD Your PR description currently looks like this 👇
While it should ideally have been like this 👇
You can remove those initial placeholders and put your own description and issue number under the appropriate sections when you open a PR. Try it next time you put up a PR 😉 |
/au |
Noted |
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.
Thanks for the review @AnimeshKumar923. LGTM!
@ALIPHATICHYD I noticed there are changes in package.lock, can you get rid of that? |
Done ☑️ |
@ALIPHATICHYD there seems to be an error, your changes are not rendering :/ |
I will try resolving it from my end. |
</a> | ||
</div> | ||
</div> | ||
); | ||
} | ||
|
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.
Hey @ALIPHATICHYD I guess you have not exported the new formed Subscription
Component, thats why the error
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.
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.
LGTM!
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.
LGTM!
Nice one @ALIPHATICHYD ! |
The subscription component name was misspelled as "Subcription". This commit fixes the typo and updates the component styles to improve the layout on different screen sizes.
Description
Related issue(s)
Resolves #342