Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

feat: Added sidebar component to community page - #1174

Merged
benhalverson merged 4 commits into
nodejs:masterfrom
lancemccluskey:1145-community-page-sidebar-component
Mar 25, 2021
Merged

feat: Added sidebar component to community page#1174
benhalverson merged 4 commits into
nodejs:masterfrom
lancemccluskey:1145-community-page-sidebar-component

Conversation

@lancemccluskey

Copy link
Copy Markdown
Contributor

Description

Added sidebar component in pages/community.tsx based on figma design here.

Related Issues

Fixes #1145

@lancemccluskey lancemccluskey left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

/preview

@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Mar 24, 2021
@github-actions github-actions Bot removed the create-preview Generate preview on staging.nodejs.dev label Mar 24, 2021
@github-actions

Copy link
Copy Markdown

Please find a preview at: https://staging.nodejs.dev/1174/

@mikeesto mikeesto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Direct preview link: https://staging.nodejs.dev/1174/community/

Looks good! 👍

Comment thread src/pages/community.tsx Outdated
@manishprivet

Copy link
Copy Markdown
Member

Clicking on section links redirects to learn section. Is that the supposed behavior?

Also, each section redirects to a heading on the main community page. Can we try making different pages/MD files for each section?

@lancemccluskey

Copy link
Copy Markdown
Contributor Author

I dont think that was in scope for this ticket. This was just to make the sidebar component. I assumed those would be in a future issue. @benhalverson would have more insight though

@benhalverson

Copy link
Copy Markdown
Member

Yes thats right @lancemccluskey

@designMoreWeb designMoreWeb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #1174 (a124145) into master (8d9753b) will decrease coverage by 0.61%.
The diff coverage is 28.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1174      +/-   ##
==========================================
- Coverage   51.35%   50.73%   -0.62%     
==========================================
  Files          55       55              
  Lines         664      676      +12     
  Branches      192      197       +5     
==========================================
+ Hits          341      343       +2     
- Misses        316      326      +10     
  Partials        7        7              
Impacted Files Coverage Δ
src/pages/community.tsx 0.00% <0.00%> (ø)
src/components/NavigationItem/index.tsx 75.00% <80.00%> (-2.78%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d9753b...a124145. Read the comment docs.

@benhalverson

Copy link
Copy Markdown
Member

Can you look into why we are getting a conflict @lancemccluskey

@lancemccluskey

Copy link
Copy Markdown
Contributor Author

Yeah I'll take a look

@lancemccluskey

Copy link
Copy Markdown
Contributor Author

@benhalverson @designMoreWeb I added a few tests since I added a prop to the NavigationItem component. Im not seeing any conflicts now Ben. Or were you referring to the code coverage stuff?

@benhalverson

Copy link
Copy Markdown
Member

Screen Shot 2021-03-24 at 7 28 44 PM
I see this error when I try to rebase and merge

@lancemccluskey

Copy link
Copy Markdown
Contributor Author

Ahhh okay that makes more sense now. Ill fix the conflicts

@lancemccluskey

Copy link
Copy Markdown
Contributor Author

Okay I following the directions for that issue in a stackoverflow I found here so hopefully its all good on your end.

@benhalverson

Copy link
Copy Markdown
Member

nope still the same message 😢

@lancemccluskey
lancemccluskey force-pushed the 1145-community-page-sidebar-component branch from 8b34271 to b1c3bc5 Compare March 25, 2021 10:18
@lancemccluskey

Copy link
Copy Markdown
Contributor Author

Whoops I accidentally pulled the old commits into my branch instead of pushing the new stuff after rebasing. NOW everything should be fine lol. Do you think well ever switch to just using a clone of the repo instead of forks? I honestly dont understand the benefit of using forks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feat: Sidebar component for /community page

6 participants