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

refactor(protocol-designer): add some styling to navigation bar #15972

Merged
merged 5 commits into from
Aug 13, 2024

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Aug 12, 2024

closes AUTH-624

Overview

add some temporary styling to the nav bar and rename to navigationBar

Test Plan and Hands on Testing

turn on the redesign feature flag and look at the navigation after clicking on "create new protocol". It is not final but wanted to add a bit of design. We will have to go back and add the breadcrumbs and remove the other routes from the nav, it would be good to work on that after the Protocol Overview page is a bit developed

Changelog

refactor navigation slightly so there is some styling

Review requests

see test plan

Risk assessment

low

@jerader jerader requested a review from a team as a code owner August 12, 2024 18:23
@jerader jerader requested review from koji and ncdiehl11 and removed request for a team August 12, 2024 18:23
import { actions as loadFileActions } from './load-file'
import type { ThunkDispatch, RouteProps } from './types'

export function NavigationBar({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i did try naming this Navigation but got confusing since react-router-dom has navigate. SOOO decided to add bar to the end so at least its a unique name to the nav bar in the app

const NavbarLink = styled(NavLink)`
color: ${COLORS.black90};
text-decoration: none;
align-self: ${ALIGN_STRETCH};
Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2024-08-13 at 8 02 49 AM

Suggested change
align-self: ${ALIGN_STRETCH};
align-self: ${ALIGN_CENTER};

or (if need to keep stretch

<Flex alignItems={ALIGN_CENTER}>
              <NavbarLink key="createNew" to={'/createNew'}>
                <StyledText desktopStyle="bodyDefaultRegular">
                  {t('create_new_protocol')}
                </StyledText>
              </NavbarLink>
            </Flex>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks! i couldn't figure out what was happening, didn't see this align-self i added

Copy link
Contributor

@koji koji left a comment

Choose a reason for hiding this comment

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

left a couple of comments on styling but overall looks good!
new create protocol's ui is sick!

@jerader jerader merged commit 3a22895 into edge Aug 13, 2024
14 checks passed
@jerader jerader deleted the pd_navigation branch August 13, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants