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

adding the reusable sidebar #411

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Dprof-in-tech
Copy link
Contributor

closes #407

@djeck1432 djeck1432 marked this pull request as ready for review December 20, 2024 08:44
@whateverfw
Copy link
Collaborator

@Dprof-in-tech seems like it is not finished yet? or can I review already?

@Dprof-in-tech
Copy link
Contributor Author

Dprof-in-tech commented Dec 20, 2024 via email

Copy link
Collaborator

@whateverfw whateverfw left a comment

Choose a reason for hiding this comment

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

@Dprof-in-tech Please resolve comments
Also sync with main. Notice that there were changes to folder structure and file's locations. Please merge it to your branch and make sure your changes is not overriden and applied correctly

<div className="tab-indicator-container">
<div className={`tab-indicator ${isCollateralActive ? 'collateral' : 'borrow'}`} />
<div className="dashboard">
<Sidebar title={'Dashboard'} items={dashboardItems} className="sidebar-docs-sticky" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

dashboard should not have title (just like in figma)

@@ -89,7 +91,8 @@ const Documentation = () => {

return (
<div className="documentation-page">
<TableOfContents tabelTitle={"Table Of Content"} defaultActiveId={"introduction"} headerHeight={80} items={tableOfContents} />
{/* <TableOfContents tabelTitle={"Table Of Content"} defaultActiveId={"introduction"} headerHeight={80} items={tableOfContents} /> */}
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove commented parts

Copy link
Collaborator

Choose a reason for hiding this comment

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

item's color do not match figma, should be #83919F in default state and #F0F0F0 in active state

also please make sure left padding and vertical padding between items are according to figma

return (
<div className="layout">
<aside className="sidebar">
{/* <aside className="sidebar">
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove commented code

id: 'home',
name: 'Home',
link: '/',
icon: '•'
Copy link
Collaborator

Choose a reason for hiding this comment

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

image

Also, title styles do not match Figma

@@ -12,9 +13,10 @@ const Documentation = () => {


const tableOfContents = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

image
  • Icorrect title styles
  • no icons near the items and seems like sub-items do not work correctly
  • when clicking Overview I can't select any other item anymore
  • when I click getting started, all items below is highlighted, also those items can't be directly selected

@Dprof-in-tech
Copy link
Contributor Author

Dprof-in-tech commented Dec 21, 2024 via email

@djeck1432
Copy link
Owner

@Dprof-in-tech Please, resolve conflict

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.

[FRONTEND] Reusable Sidebar component
3 participants