added new tab option too navbaritem #1960
Conversation
0xSpecter
requested review from
Snorre98,
andsamfu and
robines
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for opening navbar links in a new tab by introducing a new_tab prop to the NavbarItem component. The change enables navbar items to open in new browser tabs when the option is enabled.
- Added optional
new_tabboolean prop to NavbarItemProps type - Implemented conditional target attribute on Link component based on
new_tabprop value - Applied the new tab functionality to the restaurant (sulten) navbar item
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| NavbarItem.tsx | Added new_tab prop to type definition and component, implemented conditional target attribute |
| Navbar.tsx | Applied new_tab=true to the restaurant navbar item |
| Navbar.module.scss | Increased box-shadow opacity for navbar styling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| <div className={itemClasses} ref={clickOutsideRef}> | ||
| <Link to={route} className={isDesktop ? styles.navbar_link : styles.popup_link_mobile} onClick={handleClick}> | ||
| <Link | ||
| target={new_tab ? '_blank' : ''} |
There was a problem hiding this comment.
When using target=\"_blank\", you should also add rel=\"noopener noreferrer\" to prevent security vulnerabilities. The new page can access the original page's window object and potentially redirect it to a malicious site.
| target={new_tab ? '_blank' : ''} | |
| target={new_tab ? '_blank' : ''} | |
| rel={new_tab ? 'noopener noreferrer' : undefined} |
Copilot uses AI. Check for mistakes.
| route: string; | ||
| label: string; | ||
| icon?: string; | ||
| new_tab?: boolean; |
There was a problem hiding this comment.
We prefer using camelCase for js variables
| z-index: 12; | ||
| white-space: nowrap; | ||
| box-shadow: 0 0 25px 5px rgba(0, 0, 0, 0.06); | ||
| box-shadow: 0 0 25px 5px rgba(0, 0, 0, 0.1); |
There was a problem hiding this comment.
Can this change be left out of this PR?
2 participants
No description provided.