-
-
Notifications
You must be signed in to change notification settings - Fork 740
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
feat: new community section from community branch #917
Conversation
✅ Deploy Preview for asyncapi-website ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
@AceTheCreator, the PR for community section is initiated again and you can now track the changes made in the branch, with respect to the Regarding resolving the conflicts, I would tag @magicmatatjahu @derberg for this. Which method we should use, directly make a merge commit to the |
any is good, really, it is merged into feature branch anyway, so if something is broken with solving conflicts, it can also be fixed as a followup. I say, if easier for you is to do it directly to |
@derberg what happened to the previous PR that got merged into the community branch? |
@AceTheCreator it |
Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>
Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>
Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>
Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>
Co-authored-by: Khuda Dad Nomani <32505158+KhudaDad414@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Anisat Akinbani <52763841+Annysah@users.noreply.github.com> Co-authored-by: Anurag Goswami <64116092+Anurag607@users.noreply.github.com> Co-authored-by: V Thulisile Sibanda <66913810+thulieblack@users.noreply.github.com> Co-authored-by: Alejandra Quetzalli <alejandra.olvera.novack@gmail.com> Co-authored-by: Amisha Kumari <amishakumari544@gmail.com> Co-authored-by: asyncapi-bot <bot+chan@asyncapi.io> Co-authored-by: Christophe Dujarric <christophe@bump.sh> Co-authored-by: Martin F <gabel@users.noreply.github.com> Co-authored-by: Nelson <iamnelsonmichael@gmail.com> Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>
Co-authored-by: Khuda Dad Nomani <32505158+KhudaDad414@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Anisat Akinbani <52763841+Annysah@users.noreply.github.com> Co-authored-by: Anurag Goswami <64116092+Anurag607@users.noreply.github.com> Co-authored-by: V Thulisile Sibanda <66913810+thulieblack@users.noreply.github.com> Co-authored-by: Alejandra Quetzalli <alejandra.olvera.novack@gmail.com> Co-authored-by: Amisha Kumari <amishakumari544@gmail.com> Co-authored-by: asyncapi-bot <bot+chan@asyncapi.io> Co-authored-by: Christophe Dujarric <christophe@bump.sh> Co-authored-by: Martin F <gabel@users.noreply.github.com> Co-authored-by: Nelson <iamnelsonmichael@gmail.com> Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>
the thing is that when you go to
there are some additional community items that are not promoted on landing page so in short I think the idea behind makes sense? not a must-have for me in this PR but if we have an agreement we should definitely have a dedicated followup issue on that |
I understand now. But i thought keeping the landing page simple and straightforward was a really good approach. I can create a separate issue and make someone contribute to it :) What do you think @alequetzalli? |
Uh oh... do I have to answer this one? 😄 Can't I just agree with everyone? 😂😂😂😂 LOL kidding... Ok, so tbh here's my take:
|
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 @AceTheCreator, here's my review...
I see that lot of images are being added via this PR and they are in various formats like PNG
, JPEG
or JPG
. Kindly make these images compressed (if possible) and in the format of either SVG
or WEBP
. You can use https://squoosh.app/ to convert the images.
Also, I see 2 images of Fran being added, both are same, just different in terms of format of the images. Kindly have only 1 image of each person.
components/layout/CommunityLayout.js
Outdated
import React from 'react'; | ||
import Head from '../Head'; | ||
import Container from './Container'; | ||
import NavBar from '../navigation/NavBar'; | ||
import StickyNavbar from '../navigation/StickyNavbar'; | ||
|
||
|
||
export default function CommunityLayout({ | ||
title, | ||
description, | ||
children, | ||
wide = true, | ||
}) { | ||
return ( | ||
<> | ||
<Head title={title} description={description} /> | ||
<StickyNavbar> | ||
<NavBar className="max-w-screen-xl block px-4 sm:px-6 lg:px-8 mx-auto" /> | ||
</StickyNavbar> | ||
<Container wide={wide}>{children}</Container> | ||
</> | ||
); | ||
} |
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.
import React from 'react'; | |
import Head from '../Head'; | |
import Container from './Container'; | |
import NavBar from '../navigation/NavBar'; | |
import StickyNavbar from '../navigation/StickyNavbar'; | |
export default function CommunityLayout({ | |
title, | |
description, | |
children, | |
wide = true, | |
}) { | |
return ( | |
<> | |
<Head title={title} description={description} /> | |
<StickyNavbar> | |
<NavBar className="max-w-screen-xl block px-4 sm:px-6 lg:px-8 mx-auto" /> | |
</StickyNavbar> | |
<Container wide={wide}>{children}</Container> | |
</> | |
); | |
} |
This file has been created but not used anywhere in the repo and it already has the same changes as in GenericLayout
. Kindly remove this file as it is not needed.
pages/community/ambassadors/index.js
Outdated
const data = [ | ||
{ | ||
title: 'Written content', | ||
details: | ||
'Write guides, step-by-step tutorials, community documentation, AsyncAPI blog posts, and beyond.', | ||
icon: '/img/illustrations/blog.svg', | ||
}, | ||
{ | ||
title: 'Video content', | ||
details: | ||
'Produce educational videos on YouTube and other platforms for AsyncAPI.', | ||
icon: '/img/illustrations/video-creation.svg', | ||
}, | ||
{ | ||
title: 'Live streams', | ||
details: 'Moderate or host live streams that demo the AsyncAPI ecosystem.', | ||
icon: '/img/illustrations/live.svg', | ||
}, | ||
{ | ||
title: 'Give talks', | ||
details: | ||
'Speak at meetups and conferences; we’ll help with slides, abstract submissions, and travel budget.', | ||
icon: '/img/illustrations/speaking.svg', | ||
}, | ||
{ | ||
title: 'Interactive Learning', | ||
details: | ||
'Gamify educational content and create interactive learning paths for teaching AsycnAPI and event-driven architectures.', | ||
icon: '/img/illustrations/learning-app.svg', | ||
}, | ||
{ | ||
title: 'Build real-life usecases example', | ||
details: | ||
'Develop real-life usecase project example using the AsyncaAPI specification', | ||
icon: '/img/illustrations/codes.svg', | ||
}, | ||
{ | ||
title: 'AsyncAPI Contributions', | ||
details: | ||
'Collaborate with the AsyncAPI community via diverse contributions and improvements.', | ||
icon: '/img/illustrations/advisor.svg', | ||
}, | ||
{ | ||
title: 'Gather Use-Cases', | ||
details: | ||
'Collect data from existing AsyncAPI users and create use-case studies.', | ||
icon: '/img/illustrations/meeting.jpg', | ||
}, | ||
]; | ||
|
||
|
||
const tokens = [ | ||
{ | ||
emoji: '🗺️', | ||
title: 'Travel', | ||
details: 'Ambassadors are provided free entry to AsyncAPI conferences.', | ||
}, | ||
{ | ||
emoji: '🌟', | ||
title: 'Recognition', | ||
details: 'Ambassadors receive community-wide recognition.', | ||
}, | ||
{ | ||
emoji: '🎁', | ||
title: 'Special Swags', | ||
details: | ||
'Community members recognize you by gifting you exclusive AsyncAPI Ambassador swag.', | ||
}, | ||
{ | ||
emoji: '🧰', | ||
title: 'Workshop Swags', | ||
details: | ||
'Ambassadors are gifted swag from AsyncAPI conferences and workshops.', | ||
}, | ||
]; |
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.
Can we include this array of data inside another json file as we are getting huge chunks of lines inside this file and later on, it will be difficult for us to extend the page further?
pages/community/index.js
Outdated
/> | ||
</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.
I see that you are making a separate variable to render this block. Will you think of making this as a separate component?
Also, I see that most of the files inside pages/communtiy
has huge lines of code. Can we think of making separate components for them as we are also looking for making UI tests on website?
* replaced image format * removed unused component * moved ambassadors content to a new file * created components for community section * changes implemented * . * added shadow to cards and bd color * added suggested changes * .
Any other suggestions? cc @derberg @akshatnema @alequetzalli |
@AceTheCreator can you fix merge conflicts first 🙏🏼 |
@akshatnema your help is needed :) |
Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>
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.
It's not possible for me to review this PR as a whole from the codebase point of view as there are around 2k lines of changes. So, we will make sure we won't be dealing with any such PR in the future and will review each individual PR despite having the thoughts that we can improve it later.
Here's my another round of review:
The videos in the list are not sorted according to the ascending order of dates, like the meeting of 20th March should be displayed first and then further meetings should be displayed according to the dates.
{ icon: IconDashboard, title: 'Dashboard',href: '/community/dashboard', description: `Just need a good first issue to start your contribution journey? or want to see what topics are hot in discussion?`}, | ||
{ icon: IconMeetings, title: 'Meetings', href: '/community/meetings', description: 'See what meetings are organized under AsyncAPI umbrella and join one of them.' }, | ||
{ icon: IconMeetings, title: 'Meetings', href: '/community/events', description: 'See what meetings are organized under AsyncAPI umbrella and join one of them.' }, |
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.
Should the titile for this option be changed to something else for this page now, as we are not only restricted to Meetings but rather we are more focused on the events too like AsyncAPI Conference?
WDYT @derberg @alequetzalli
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.
It's best if we rename the title to "Events"
@akshatnema you're right akshat |
@akshatnema I disagree with this approach, as it only prolongs this PR. So I'll advise you to create the review once, and whatever thing you find out later on can be opened as a GFI. Your approach will only keep making me open more PR fixing small changes |
Sorry, but actually you misunderstood my concern slightly. I'm not asking to make new PRs to make small changes in this PR. But what I said that when you had initialized individual PRs for the pages of the community section previously, I must have made a complete review for that PR instead of waiting for them to merge in the community branch. Anyways, my reviews are done. Approved ✔️. Waiting for the approval from @derberg @alequetzalli |
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.
@alequetzalli we need your final approval
@alequetzalli, any other suggestion from your end? Before we go ahead merge this PR |
@AceTheCreator Will you like to merge the PR? Since, you are the major contributor of this PR and I want you to take this pleasure. Just do the |
/rtm |
Description
This PR adds a new page to the website which describes all the community-related information and announcements in it. This PR is made draft to track the changes done in the
community
branch. Whoever wants to contribute to this issue/PR has to make a branch fromcommunity
branch to his forked repo.Related issue(s)
Resolves #903