fix: responsive issues related to community hamburger in navbar #730

Mcnoble1 · 2022-05-03T06:14:38Z

Description
Fixed the community dropdown in the Navigation bar to match the others.

Related issue(s)
Fixes #724

@mcturco

…aders

…reenreaders

…ons in slack section for improved accessibility

…slack section for improved accessibility

…site into accessibility-fixes

netlify · 2022-05-03T06:14:44Z

✅ Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`9f9e3c3`
🔍 Latest deploy log	https://app.netlify.com/sites/asyncapi-website/deploys/6284b4dac3448b0008098645
😎 Deploy Preview	https://deploy-preview-730--asyncapi-website.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

github-actions · 2022-05-03T06:21:15Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🔴 Performance	42
🟢 Accessibility	98
🟢 Best practices	100
🟢 SEO	100
🔴 PWA	30

Lighthouse ran on https://deploy-preview-730--asyncapi-website.netlify.app/

mcturco · 2022-05-03T20:11:43Z

Hi @Mcnoble1 !
Thanks for raising this PR. I just checked out your PR and locally added the new icons that I have created for the community dropdown on my local server. Here is a preview:

Let me know if it is okay to push these commits to the PR. Thanks! cc: @akshatnema

Mcnoble1 · 2022-05-03T20:15:39Z

This is super nice @mcturco. Neat and communicates the message. Do push the commits please.

into community-dropdown

Mcnoble1 · 2022-05-04T11:35:32Z

@akshatnema. I've made the necessary updates to mobile view and removed the commits from accessibility-fixes

akshatnema

Don't use the SVG code of the icons directly as a component. Instead, add them as an image in the repository and then use them.

akshatnema · 2022-05-05T10:55:29Z

components/icons/Contributing.js

@@ -0,0 +1,25 @@
+export default function IconContributing ({ className }) {


The icons added for the menu should not be added as the SVG code inside components. All the illustrations used should be added inside /public/img/illustrations/icons/ folder and not in the form of code, but in the SVG format. Remove this icons folder inside the components folder and place the files under the right directory. Also, don't forget to change the path of icons in the CommunityItems.js.

For the icons, I met them in the components/icons folder. I wasn't the one that created them there. So I'm not sure I will want to change the location since I didn't make the folder structure.

Actually, the components/icons folder has been made to structures some special types of icons like GitHub.js, Modelina.js, Parser.js, etc. whereas the type of SVG icons are directly added to the /public/img/illustrations/icons/ in order to use it. So, it will be better you directly export the SVG icons to the above directory and then use it.

I also ask @magicmatatjahu here if the present addition of icons are correct or it should be under public folder.

Please do ask.

I think the way @Mcnoble1 implemented the icons is correct as this is the behavior for the other SVG icons in the nav menu. I would refrain from restructuring the files in this PR, if there is a better way to handle the files we should suggest that idea in a separate issue. But of course, looking at @magicmatatjahu for final say on code changes 😄

I always use icons (svg) as React component, so I prefer to change them to the JS function rather to have them as normal svgs - remember that we applied in some places styles for them, so it's easier to treat them as components.

akshatnema · 2022-05-05T10:58:36Z

components/navigation/communityItems.js

Don't change the name of the icons (inside the text) as such. Revert the names changed by you and add the path of the icons in the icon field.

I really do not get this. I simply followed the naming convention used in navigation/learningItems.js and navigation/toolingItems.js which are the other navbar items.

Sorry that I wrongly get it in the first instance. What I want to propose here is to use the camelCase format for writing the variable names here because we usually use the camelCase format to write the names.

The icons here are not variable names but function names.

I know that they are function names, but what I see in the repository is that camelCase is preferable. So, I suggested you for that. Let's wait for the view of code owners towards this.

I don't understand 😅 The IconTools is a function (React component) so now it's good.

akshatnema

Added a few more comments to remove certain code added in the PR.

akshatnema · 2022-05-05T14:44:07Z

components/navigation/MenuBlocks.js

              rel={isExternalHref ? "noopener noreferrer" : undefined}
            >
              <div className={`flex-shrink-0 flex items-center justify-center h-10 w-10 rounded-lg border border-gray-800 bg-secondary-100 text-gray-900 sm:h-12 sm:w-12 ${item.comingSoon && 'opacity-50'}`}>
-                <item.icon className="h-6 w-6" />
+                <item.icon className="h-8 w-8" />


Any specific reason why the size of the icons is increased?

I think it's for more visibility. @mcturco suggested we do that in the comment section above. She'll have her reasons for that.

Yep like @Mcnoble1 said, I increased the size of all the icons so that you can see the detail better. Eventually we will replace the icons in the other menus as well to match the new icons in this PR 😄

akshatnema · 2022-05-05T14:50:20Z

components/navigation/NavBar.js

@@ -53,6 +52,8 @@ export default function NavBar ({
            <svg className="h-6 w-6" stroke="currentColor" fill="none" viewBox="0 0 24 24">
              <title>Menu</title>
              <path strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" d="M4 6h16M4 12h16M4 18h16" />
+              <path strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" d="M4 6h16M4 12h16M4 18h16"/>


The 2 lines added here are not needed as they overwrite the existence of 3 parallel lines of Menu. You can remove this.

I've removed them.

akshatnema · 2022-05-05T14:51:14Z

components/navigation/NavBar.js

@@ -69,7 +70,8 @@ export default function NavBar ({

          <div className="relative">
            <NavItem text="Community" onClick={() => showMenu('community')} hasDropdown />
-            {open === 'community' && <NavMenu items={communityItems} />}
+            {/* {open === 'community' && <NavMenu items={communityItems} />} */}


If certain piece of code is not needed, you can remove it instead of commenting it. Please do remove this comment.

Yikes, must have been an oversight, thanks. removed.

akshatnema · 2022-05-05T14:52:25Z

package.json

@@ -60,6 +60,8 @@
    "react-dom": "^17.0.2",
    "react-ga": "^3.1.2",
    "react-gtm-module": "^2.0.11",
+    "react-helmet": "^6.1.0",


Can you explain why these 2 new dependencies are needed in this PR?

Another oversight would have happened while I was reverting some commits. I've removed them. Thanks so much @akshatnema

into community-dropdown

akshatnema · 2022-05-06T05:58:36Z

components/navigation/communityItems.js

+  { icon: IconTools, title: 'Tools & Services', href: '/docs/community/tooling', description: 'Explore the tools and services our awesome community has created.' },
+  { icon: IconGithubOrganization, title: 'Github Organization', href: 'https://github.com/asyncapi', target: '_blank', description: 'Want to sneak in the code? Everything we do is open-sourced in our Github organization.' },
+  { icon: IconSlack, title: 'Slack Workspce', href: 'https://asyncapi.com/slack-invite', target: '_blank', description: `Need help? Want to share something? Join our Slack workspace. We're nice people :)` },


Correct the spelling of Workspace here.

Done. Thank you.

magicmatatjahu

@Mcnoble1 Sorry for such a delay! Awesome work btw! I have some comments, please refer to them and also I found one "bug":

I think that we should try to fix these indendations in mobile views:

Learning group has bigger padding that other and it doesn't look nice.

magicmatatjahu · 2022-05-17T09:47:52Z

package-lock.json

+<<<<<<< HEAD
+        "react-helmet": "^6.1.0",
+<<<<<<< HEAD
+        "react-router-hash-link": "^2.4.3",
+=======
+>>>>>>> parent of 14d36f9... fix: add html lang attribute for improved accessibility
+=======
+>>>>>>> parent of 5e19416... feat: add skip link to the navbar


Fix these leftover conflicts

@magicmatatjahu, will do that now. Thank you.

Mcnoble1 · 2022-05-17T10:36:50Z

@magicmatatjahu all fixed now.

magicmatatjahu

LGTM! @akshatnema @mcturco Do you wanna add something before merge?

magicmatatjahu · 2022-05-18T09:06:12Z

/rtm

Mcnoble1 and others added 22 commits April 26, 2022 19:19

fix: add alternative name for AsyncAPI logo for screenreaders

180bc2c

fix: add alternative name for footer netlify logo for screenreaders

9d6a66b

fix: add alternative name for testimonial author avatars for screenre…

5ac5d8e

…aders

fix: add alternative names for logos of companies and sponsors for sc…

2fe2386

…reenreaders

fix: add alternative text for rs for avatars, emojis, and reaction ic…

fc3ba35

…ons in slack section for improved accessibility

fix: add alternative text for avatars, emojis, and reaction icons in …

a39d242

…slack section for improved accessibility

fix: add alternative text for hamburger menu icon for screenreaders

7288474

fix: add html lang attribute for improved accessibility

14d36f9

Merge branch 'master' into accessibility-fixes

b24adac

feat: add skip link to the navbar

5e19416

fix: add html lang atttibute across all pages

bd93162

fix: add alt text for rss image

2c8ee76

Merge branch 'accessibility-fixes' of https://github.com/Mcnoble1/web…

4e66164

…site into accessibility-fixes

Merge branch 'master' into accessibility-fixes

b0233e3

Merge branch 'master' into accessibility-fixes

2c875cf

Merge branch 'master' into accessibility-fixes

3057e82

fix add skip link and accessibility issues clean up

d91c1bc

Merge branch 'accessibility-fixes' of https://github.com/Mcnoble1/web…

a14f613

…site into accessibility-fixes

fix: create community panel file for community dropdown

5a89ece

updated Navbar's community section

a3d40d5

updated community items to display in two-column structure

a561bc5

checked through the files to update community panel

92d86bd

Mcnoble1 requested review from fmvilas, derberg, magicmatatjahu and asyncapi-bot-eve as code owners May 3, 2022 06:14

Mcnoble1 added 11 commits May 4, 2022 10:36

revert accessibility PR

3f00659

revert accessibility PR

6eb36a1

revert accessibility PR

4b3bb99

revert accessibility PR

5cd791e

revert accessibility PR

0800a1d

removed accessibility PR from community-dropdown PR

67de7fa

Merge branch 'community-dropdown' of https://github.com/Mcnoble1/website

e9917f8

into community-dropdown

fixed log errors in package.json file

8ad0cf2

fixed revert commits in navbar

d6e3c8e

fixed revert error logs index file

c33445f

fixed community dropdown menu in mobile navigation

df61463

Merge branch 'master' into community-dropdown

20ae7cb

akshatnema requested changes May 5, 2022

View reviewed changes

Mcnoble1 added 2 commits May 5, 2022 18:47

removed redundant code

a4e8770

Merge branch 'community-dropdown' of https://github.com/Mcnoble1/website

d72f906

into community-dropdown

akshatnema reviewed May 6, 2022

View reviewed changes

corrected Worspace typo

eae0c26

magicmatatjahu requested changes May 17, 2022

View reviewed changes

Mcnoble1 added 2 commits May 17, 2022 11:11

fix: resolve conflict in package-lock.json file

346cb6e

fix: learning group padding to match others

a51a946

Merge branch 'master' into community-dropdown

db389fc

magicmatatjahu approved these changes May 17, 2022

View reviewed changes

Merge branch 'master' into community-dropdown

9f9e3c3

asyncapi-bot added the ready-to-merge label May 18, 2022

asyncapi-bot merged commit 694f9b9 into asyncapi:master May 18, 2022

akshatnema linked an issue Sep 1, 2022 that may be closed by this pull request

Responsive issues related to Hamburger menu of Community in Navbar #724

Closed

		@@ -0,0 +1,25 @@
		export default function IconContributing ({ className }) {

fix: responsive issues related to community hamburger in navbar #730

fix: responsive issues related to community hamburger in navbar #730

Conversation

Mcnoble1 commented May 3, 2022 • edited Loading

netlify bot commented May 3, 2022 • edited Loading

✅ Deploy Preview for asyncapi-website ready!

github-actions bot commented May 3, 2022 • edited Loading

mcturco commented May 3, 2022

Mcnoble1 commented May 3, 2022

Mcnoble1 commented May 4, 2022

akshatnema left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akshatnema May 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akshatnema left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magicmatatjahu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mcnoble1 commented May 17, 2022

magicmatatjahu left a comment

Choose a reason for hiding this comment

magicmatatjahu commented May 18, 2022

Mcnoble1 commented May 3, 2022 •

edited

Loading

netlify bot commented May 3, 2022 •

edited

Loading

github-actions bot commented May 3, 2022 •

edited

Loading

akshatnema May 5, 2022 •

edited

Loading