Skip to content

Commit 36e1be5

Browse files
kunalverma2512kunalverma2512
committed
[change] Eliminate redundancy of header HTML #314 
1 parent 1d0f214 commit 36e1be5

3 files changed

Lines changed: 98 additions & 175 deletions

File tree

‎client/components/header/header.js‎

Lines changed: 63 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,62 @@ export default class Header extends React.Component {
7676
const {logo, links, second_logo: secondLogo} = header;
7777
const {pathname} = location;
7878
const internalLinks = [`/${orgSlug}/login`, `/${orgSlug}/registration`];
79+
80+
// DRY abstraction: We create the language buttons once and reuse them.
81+
const languageButtons = languages.map((lang) => (
82+
<button
83+
type="button"
84+
className={`${
85+
language === lang.slug ? "active " : ""
86+
}header-language-btn header-language-btn-${lang.slug}`}
87+
key={lang.slug}
88+
onClick={() => setLanguage(lang.slug)}
89+
>
90+
{lang.text}
91+
</button>
92+
));
93+
94+
// DRY abstraction: We map the complex links array only once.
95+
const navLinks =
96+
links &&
97+
links.map((link, index) => {
98+
if (!shouldLinkBeShown(link, isAuthenticated, userData)) {
99+
return null;
100+
}
101+
if (
102+
isInternalLink(link.url) &&
103+
(internalLinks.indexOf(link.url) < 0 || !isAuthenticated)
104+
) {
105+
return (
106+
<Link
107+
className={`header-link header-link-${index + 1} ${
108+
pathname === link.url.replace("{orgSlug}", orgSlug)
109+
? "active"
110+
: ""
111+
} button `}
112+
to={link.url.replace("{orgSlug}", orgSlug)}
113+
key={index}
114+
>
115+
{getText(link.text, language)}
116+
</Link>
117+
);
118+
}
119+
return (
120+
<a
121+
href={link.url}
122+
className={`header-link header-link-${index + 1} button`}
123+
target="_blank"
124+
rel="noreferrer noopener"
125+
key={link.url}
126+
>
127+
{getText(link.text, language)}
128+
</a>
129+
);
130+
});
131+
79132
return (
80133
<>
81-
<div className="header-container header-desktop">
134+
<div className="header-container">
82135
<div className="header-row-1">
83136
<div className="header-row-1-inner">
84137
<div className="header-left">
@@ -88,7 +141,7 @@ export default class Header extends React.Component {
88141
<img
89142
src={getAssetPath(orgSlug, logo.url)}
90143
alt={logo.alternate_text}
91-
className="header-logo-image header-desktop-logo-image"
144+
className="header-logo-image"
92145
/>
93146
</Link>
94147
) : null}
@@ -100,97 +153,14 @@ export default class Header extends React.Component {
100153
<img
101154
src={getAssetPath(orgSlug, secondLogo.url)}
102155
alt={secondLogo.alternate_text}
103-
className="header-logo-image header-desktop-logo-image"
156+
className="header-logo-image"
104157
/>
105158
</div>
106159
)}
107160

108161
<div className="header-right">
109-
{languages.map((lang) => (
110-
<button
111-
type="button"
112-
className={`${
113-
language === lang.slug ? "active " : ""
114-
}header-language-btn header-desktop-language-btn header-language-btn-${
115-
lang.slug
116-
}`}
117-
key={lang.slug}
118-
onClick={() => setLanguage(lang.slug)}
119-
>
120-
{lang.text}
121-
</button>
122-
))}
123-
</div>
124-
</div>
125-
</div>
126-
<div className="header-row-2">
127-
<div className="header-row-2-inner">
128-
{links &&
129-
links.map((link, index) => {
130-
if (!shouldLinkBeShown(link, isAuthenticated, userData)) {
131-
return null;
132-
}
133-
if (
134-
isInternalLink(link.url) &&
135-
(internalLinks.indexOf(link.url) < 0 || !isAuthenticated)
136-
) {
137-
return (
138-
<Link
139-
className={`header-link header-desktop-link
140-
header-link-${index + 1} ${
141-
pathname === link.url.replace("{orgSlug}", orgSlug)
142-
? "active"
143-
: ""
144-
} button `}
145-
to={link.url.replace("{orgSlug}", orgSlug)}
146-
key={index}
147-
>
148-
{getText(link.text, language)}
149-
</Link>
150-
);
151-
}
152-
return (
153-
<a
154-
href={link.url}
155-
className={`header-link header-desktop-link
156-
header-link-${index + 1} button`}
157-
target="_blank"
158-
rel="noreferrer noopener"
159-
key={link.url}
160-
>
161-
{getText(link.text, language)}
162-
</a>
163-
);
164-
})}
165-
</div>
166-
</div>
167-
</div>
168-
<div className="header-mobile ">
169-
<div className="header-row-1">
170-
<div className="header-row-1-inner">
171-
<div className="header-left">
172-
<div className="header-logo-div">
173-
{logo && logo.url ? (
174-
<Link to={`/${orgSlug}`}>
175-
<img
176-
src={getAssetPath(orgSlug, logo.url)}
177-
alt={logo.alternate_text}
178-
className="header-logo-image header-mobile-logo-image"
179-
/>
180-
</Link>
181-
) : null}
182-
</div>
183-
</div>
184-
{secondLogo && (
185-
<div className="header-logo-2">
186-
<img
187-
src={getAssetPath(orgSlug, secondLogo.url)}
188-
alt={secondLogo.alternate_text}
189-
className="header-logo-image header-mobile-logo-image"
190-
/>
191-
</div>
192-
)}
193-
<div className="header-right">
162+
<div className="desktop-languages">{languageButtons}</div>
163+
194164
<div
195165
role="button"
196166
tabIndex={0}
@@ -206,58 +176,13 @@ export default class Header extends React.Component {
206176
</div>
207177
</div>
208178
</div>
179+
209180
<div
210-
className={`${menu ? "display-flex" : "display-none"} header-mobile-menu`}
181+
className={`header-row-2 ${menu ? "display-flex" : "mobile-hidden"}`}
211182
>
212-
{links &&
213-
links.map((link, index) => {
214-
if (shouldLinkBeShown(link, isAuthenticated, userData)) {
215-
if (isInternalLink(link.url)) {
216-
return (
217-
<Link
218-
className={`header-link mobile-link
219-
header-link-${index + 1} ${
220-
pathname === link.url.replace("{orgSlug}", orgSlug)
221-
? "active"
222-
: ""
223-
} button`}
224-
to={link.url.replace("{orgSlug}", orgSlug)}
225-
key={index}
226-
>
227-
{getText(link.text, language)}
228-
</Link>
229-
);
230-
}
231-
return (
232-
<a
233-
href={link.url}
234-
className={`header-link mobile-link
235-
header-link-${index + 1} button`}
236-
target="_blank"
237-
rel="noreferrer noopener"
238-
key={link.url}
239-
>
240-
{getText(link.text, language)}
241-
</a>
242-
);
243-
}
244-
return null;
245-
})}
246-
<div className="mobile-languages-row">
247-
{languages.map((lang) => (
248-
<button
249-
type="button"
250-
className={`${
251-
language === lang.slug ? "active " : ""
252-
}header-language-btn header-mobile-language-btn header-language-btn-${
253-
lang.slug
254-
}`}
255-
key={lang.slug}
256-
onClick={() => setLanguage(lang.slug)}
257-
>
258-
{lang.text}
259-
</button>
260-
))}
183+
<div className="header-unified-nav header-mobile-menu">
184+
{navLinks}
185+
<div className="mobile-languages-row">{languageButtons}</div>
261186
</div>
262187
</div>
263188
</div>

‎client/components/header/header.test.js‎

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ describe("<Header /> rendering", () => {
105105
},
106106
];
107107
wrapper = shallow(<Header {...props} />);
108-
expect(isInternalLink).toHaveBeenCalledTimes(6);
108+
expect(isInternalLink).toHaveBeenCalledTimes(3);
109109
expect(isInternalLink).toHaveBeenCalledWith("/default/login");
110110
});
111111
it("should render without authenticated links when not authenticated", () => {
@@ -151,27 +151,17 @@ describe("<Header /> rendering", () => {
151151
expect(linkText).not.toContain("link-4");
152152
});
153153
it("should render 2 links", () => {
154-
expect(wrapper.find(".header-desktop-link")).toHaveLength(2);
154+
expect(wrapper.find(".header-link")).toHaveLength(2);
155155
});
156-
it("should render 2 languages", () => {
157-
expect(wrapper.find(".header-desktop-language-btn")).toHaveLength(2);
156+
it("should render 4 languages", () => {
157+
expect(wrapper.find(".header-language-btn")).toHaveLength(4);
158158
});
159159
it("should render english as default language", () => {
160-
expect(
161-
wrapper.find(
162-
".header-desktop-language-btn.header-language-btn-en.active",
163-
),
164-
).toHaveLength(1);
165-
expect(
166-
wrapper.find(
167-
".header-desktop-language-btn.header-language-btn-it.active",
168-
),
169-
).toHaveLength(0);
160+
expect(wrapper.find(".header-language-btn-en.active")).toHaveLength(2);
161+
expect(wrapper.find(".header-language-btn-it.active")).toHaveLength(0);
170162
});
171163
it("should render logo", () => {
172-
expect(
173-
wrapper.find(".header-logo-image.header-desktop-logo-image"),
174-
).toHaveLength(1);
164+
expect(wrapper.find(".header-logo-image")).toHaveLength(1);
175165
});
176166
it("should not render logo", () => {
177167
const logo = {
@@ -182,9 +172,7 @@ describe("<Header /> rendering", () => {
182172
};
183173
props = createTestProps(logo);
184174
wrapper = shallow(<Header {...props} />);
185-
expect(
186-
wrapper.find(".header-logo-image.header-desktop-logo-image"),
187-
).toHaveLength(0);
175+
expect(wrapper.find(".header-logo-image")).toHaveLength(0);
188176
});
189177
it("should render sticky msg and close it on clicking close-btn", () => {
190178
props = createTestProps({
@@ -245,13 +233,9 @@ describe("<Header /> interactions", () => {
245233
wrapper = shallow(<Header {...props} />);
246234
});
247235
it("should call setLanguage function when 'language button' is clicked", () => {
248-
wrapper
249-
.find(".header-language-btn-it.header-desktop-language-btn")
250-
.simulate("click");
236+
wrapper.find(".header-language-btn-it").at(0).simulate("click");
251237
expect(props.setLanguage).toHaveBeenCalledTimes(1);
252-
wrapper
253-
.find(".header-language-btn-it.header-mobile-language-btn")
254-
.simulate("click");
238+
wrapper.find(".header-language-btn-it").at(1).simulate("click");
255239
expect(props.setLanguage).toHaveBeenCalledTimes(2);
256240
});
257241
it("should call handleHamburger function when 'hamburger button' is clicked", () => {

‎client/components/header/index.css‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
width: 100%;
6565
min-height: 54px;
6666
}
67-
.header-row-2-inner,
67+
.header-unified-nav,
6868
.sticky-container .inner {
6969
display: flex;
7070
flex-direction: row;
@@ -84,9 +84,7 @@
8484
.header-link.active {
8585
cursor: default;
8686
}
87-
.header-mobile {
88-
display: none;
89-
}
87+
9088
.display-none {
9189
display: none;
9290
}
@@ -116,8 +114,16 @@
116114
color: inherit;
117115
}
118116

117+
.header-hamburger {
118+
display: none;
119+
}
120+
.mobile-languages-row {
121+
display: none;
122+
}
123+
119124
@media screen and (min-width: 0px) and (max-width: 767px) {
120125
.header-hamburger {
126+
display: block;
121127
width: 30px;
122128
height: 26px;
123129
cursor: pointer;
@@ -149,31 +155,39 @@
149155
.header-hamburger div:nth-child(3) {
150156
top: 10px;
151157
}
152-
.header-mobile {
153-
display: flex;
158+
159+
.desktop-languages {
160+
display: none;
161+
}
162+
.mobile-hidden {
163+
display: none !important;
164+
}
165+
166+
.header-row-2 {
154167
flex-direction: column;
155168
}
156-
.header-mobile-menu {
169+
.header-unified-nav {
157170
transition: 0.25s all ease-in-out;
158171
flex-direction: column;
159172
align-items: center;
160173
padding: 20px 0 0;
174+
width: 100%;
161175
}
162-
.header-mobile-menu > .header-link {
176+
.header-unified-nav > .header-link {
163177
margin: 6px 0;
164178
width: 90%;
165179
box-sizing: border-box;
166180
text-align: center;
167181
font-size: 15px;
168182
padding: 12px 0;
169-
}
170-
.header-desktop {
171-
display: none;
183+
margin-right: 0;
172184
}
173185
.mobile-languages-row {
186+
display: block;
174187
width: 90%;
175188
margin: 5px 0 0 0;
176189
line-height: 40px;
190+
text-align: center;
177191
}
178192
.header-language-btn.active {
179193
cursor: auto;

0 commit comments

Comments
 (0)