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

Feat/#60 create sidebar navigation #141

Merged
merged 16 commits into from
Jun 14, 2023
Merged

Conversation

njochens
Copy link
Contributor

@njochens njochens commented Jun 7, 2023

No description provided.

@njochens njochens marked this pull request as ready for review June 13, 2023 14:49
@njochens njochens force-pushed the feat/#60-create-sidebar-navigation branch from c755e02 to abfdca8 Compare June 13, 2023 15:20
njochens added 6 commits June 13, 2023 17:37
Signed-off-by: njochens <nikolas@njochens.de>
Signed-off-by: njochens <nikolas@njochens.de>
Signed-off-by: njochens <nikolas@njochens.de>
Signed-off-by: njochens <nikolas@njochens.de>
Signed-off-by: njochens <nikolas@njochens.de>
Signed-off-by: njochens <nikolas@njochens.de>
@vringar vringar force-pushed the feat/#60-create-sidebar-navigation branch from abfdca8 to 55468b0 Compare June 13, 2023 15:39
Signed-off-by: njochens <nikolas@njochens.de>
Signed-off-by: njochens <nikolas@njochens.de>
@njochens njochens requested a review from vringar June 13, 2023 18:22
Copy link
Contributor

@vringar vringar left a comment

Choose a reason for hiding this comment

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

Sorry that this is so much.
I'm trying to be more thorough than I have been in the past

Explorer/package.json Outdated Show resolved Hide resolved
Explorer/src/components/sidebar.tsx Outdated Show resolved Hide resolved
Explorer/src/app/pods/page.tsx Outdated Show resolved Hide resolved
Explorer/src/context/sidebar_context.tsx Show resolved Hide resolved
Explorer/src/context/sidebar_context.tsx Show resolved Hide resolved
Explorer/src/components/sidebar.tsx Show resolved Hide resolved
Explorer/postcss.config.js Outdated Show resolved Hide resolved
njochens and others added 8 commits June 13, 2023 20:45
Signed-off-by: njochens <62465088+njochens@users.noreply.github.com>
… in nav is not visible

Signed-off-by: njochens <nikolas@njochens.de>
Signed-off-by: njochens <62465088+njochens@users.noreply.github.com>
Signed-off-by: njochens <nikolas@njochens.de>
Signed-off-by: njochens <nikolas@njochens.de>
Signed-off-by: vringar <zabka@campus.tu-berlin.de>
);
};

export default Object.assign(Sidebar, { ...FlowbiteSidebar });
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I wish we had a comment on this line

<FlowbiteSidebar.Item
href="dashboard"
icon={HiPresentationChartLine}
active={current_page.localeCompare("dashboard") ? false : true}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I don't like this use of the ternary.
Isn't this equivalent to this line?

Suggested change
active={current_page.localeCompare("dashboard") ? false : true}
active={current_page === "dashboard"}

@vringar vringar merged commit ddbd9df into develop Jun 14, 2023
@vringar vringar deleted the feat/#60-create-sidebar-navigation branch June 14, 2023 09:30
@jandegen jandegen linked an issue Jun 14, 2023 that may be closed by this pull request
4 tasks
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.

Create sidebar navigation
2 participants