Skip to content

Conversation

@Yehorik11
Copy link

added layout heder

Implemented the layout for the header section with navigation and middle
section

BREAKING CHANGE: 🧨 Changed the structure of the `Header` component, which may require
updates to its usage in

✅ Closes:   other parts of the application. The `header` element is now wrapped
with a `div` and requires
@Tornik73
Copy link
Member

Tornik73 commented Apr 1, 2025

Fix merge conflicts

@Tornik73 Tornik73 changed the base branch from main to dev April 1, 2025 14:20
export default function Account() {
return (
<div className={styles.account}>
<p>Sign In</p>&nbsp;/&nbsp;<p>Sign Up</p>
Copy link
Member

Choose a reason for hiding this comment

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

We need to use localization

.callNow {
display: flex;
align-items: center;
.phoneNumber {
Copy link
Member

Choose a reason for hiding this comment

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

No need to use nested styles here

<div className={styles.callNow}>
<Image src={Phone} alt='phone' />
<a href='tel:2195550114' className={styles.phoneNumber}>
(219) 555-0114
Copy link
Member

Choose a reason for hiding this comment

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

should not be hardcoded, we need to allow admins to customize telephone, emails, etc.

.wrapper {
width: 100%;
background-color: $gray-800;
.nav {
Copy link
Member

Choose a reason for hiding this comment

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

remove nested styles

@@ -0,0 +1,16 @@
.smallOne,
.midle {
Copy link
Member

Choose a reason for hiding this comment

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

rename midle


import DropDown from '../../assets/images/Drop Down.svg';

export default function SmallOne() {
Copy link
Member

Choose a reason for hiding this comment

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

I do not like this name

Comment on lines 16 to 19
<div className={styles.option}>
<p>Eng</p>
<Image src={DropDown} alt='Drop Down' />
</div>
Copy link
Member

Choose a reason for hiding this comment

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

should be in separate component

Comment on lines 21 to 24
<div className={styles.option}>
<p>USD</p>
<Image src={DropDown} alt='Drop Down' />
</div>
Copy link
Member

Choose a reason for hiding this comment

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

should be in separate component


import styles from './SmallOne.module.scss';

import DropDown from '../../assets/images/Drop Down.svg';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import DropDown from '../../assets/images/Drop Down.svg';
import DropDownIcon from '../../assets/images/dropdown-icon.svg';

@@ -0,0 +1,31 @@
.smallOne {
Copy link
Member

Choose a reason for hiding this comment

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

remove nested styles
Rename smallOne to another name

@Tornik73
Copy link
Member

Tornik73 commented Apr 1, 2025

add link to the task from Jira

  rename component Midle to HeaderActions, removed nested styles file,
added new component InfoOptions

BREAKING CHANGE: 🧨   rename component Midle to HeaderActions, removed nested styles file,
added new component InfoOptions
created new component Price

BREAKING CHANGE: 🧨 created new component Price

✅ Closes: created new component Price
import styles from './CallNow.module.scss';

const phoneNumber = 2195550114;
const phoneDisplay = '(219) 555-0114';
Copy link
Member

Choose a reason for hiding this comment

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

return (
<div className={styles.callNow}>
<Image src={Phone} alt='phone' />
<a href={`tel:${phoneNumber}`} className={styles.phoneNumber}>
Copy link
Member

Choose a reason for hiding this comment

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

Create separate shared PhoneNumber component

Comment on lines +14 to +17
<div className={styles.selector}>
<InfoOption>Eng</InfoOption>
<InfoOption>USD</InfoOption>
</div>
Copy link
Member

@Tornik73 Tornik73 Apr 3, 2025

Choose a reason for hiding this comment

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

should be separate component

<InfoOption>Eng</InfoOption>
<InfoOption>USD</InfoOption>
</div>
<span className={styles.divider}>|</span>
Copy link
Member

Choose a reason for hiding this comment

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

should be separate component

fix layout header

BREAKING CHANGE: 🧨 fix layout header

✅ Closes: fix layout header
added component layout

BREAKING CHANGE: 🧨 added component layout

✅ Closes: added component layout
styles[variant],
styles[fill],
styles[size],
isHovered &&
Copy link
Member

Choose a reason for hiding this comment

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

You can remove it, and do it with css only

padding-top: 16px;

p {
padding: 0 !important;
Copy link
Member

Choose a reason for hiding this comment

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

Remove important usage

const navLinks = [
{ href: '/', label: 'Home', isOption: true, isActive: true },
{ href: '/Shop', label: 'Shop', isOption: true },
{ href: '/Pages', label: 'Pages', isOption: true },
Copy link
Member

Choose a reason for hiding this comment

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

Rename pages routes

@@ -0,0 +1,4 @@
<svg width="17" height="20" viewBox="0 0 17 20" fill="none" xmlns="http://www.w3.org/2000/svg">
Copy link
Member

Choose a reason for hiding this comment

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

Please do not use spaces in names, check all places
Rename all icons to lower case

@@ -0,0 +1,16 @@
import Image from 'next/image';
import Phone from '../../assets/images/PhoneCall.svg';
Copy link
Member

Choose a reason for hiding this comment

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

Assets importing should be in this format

// index.ts
export { ReactComponent as PhoneIcon } from './phone-icon.svg';

// Component file
import { PhoneIcon } from '@assets/icons';

<PhoneIcon/>

border-radius: 50%;
}

.activeIcon {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.activeIcon {
.active {

@@ -0,0 +1,17 @@
.socialIcons {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.socialIcons {
.icons {

];

export default function SocialMedia() {
const [hovered, setHovered] = useState<string | null>(null);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const [hovered, setHovered] = useState<string | null>(null);

Comment on lines +50 to +56
className={
hovered === name
? `${styles.activeIcon} ${styles.icon}`
: styles.icon
}
onMouseEnter={() => setHovered(name)}
onMouseLeave={() => setHovered(null)}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
className={
hovered === name
? `${styles.activeIcon} ${styles.icon}`
: styles.icon
}
onMouseEnter={() => setHovered(name)}
onMouseLeave={() => setHovered(null)}

@@ -0,0 +1,13 @@
.subscribeWrapper {
Copy link
Member

Choose a reason for hiding this comment

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

rename to wrapper

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.

3 participants