-
Notifications
You must be signed in to change notification settings - Fork 3
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
1093/search redesign #1150
1093/search redesign #1150
Conversation
…unnecessary code duplication
…mage displaying in search result cards
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Round 2 of comments
next/components/molecules/SearchPageNew/SearchResultsHeader.tsx
Outdated
Show resolved
Hide resolved
next/components/molecules/SearchPageNew/SearchResultsHeader.tsx
Outdated
Show resolved
Hide resolved
next/components/molecules/SearchPageNew/SearchResultsHeader.tsx
Outdated
Show resolved
Hide resolved
next/components/molecules/SearchPageNew/SearchResultsHeader.tsx
Outdated
Show resolved
Hide resolved
next/components/molecules/SearchPageNew/SearchResultsHeader.tsx
Outdated
Show resolved
Hide resolved
next/components/molecules/SearchPageNew/GeneralSearchResults.tsx
Outdated
Show resolved
Hide resolved
next/components/molecules/SearchPageNew/SearchResultsHeader.tsx
Outdated
Show resolved
Hide resolved
@@ -12,7 +13,9 @@ export const SearchResultsHeader = ({ title, handleShowMore }: SearchResultsHead | |||
|
|||
return ( | |||
<div className="flex flex-col flex-wrap items-baseline justify-between gap-y-2 text-gray-800 sm:flex-row"> | |||
<h2 className="text-size-h4">{title}</h2> | |||
<Typography type="h2" size="h4" className="text-gray-800"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this kind of situations, we always want confront and change Figma to follow design system rules. --> Write a comment to Figma and tag Patrik.
DS Foundations can be found here https://www.figma.com/file/ctDKMhAPIjLUVNNmq430D4/DS-ESBS%3A-Foundations?node-id=59-21&t=M4lXbCEzOWWIiekd-0
@@ -8,6 +8,7 @@ import React, { ReactNode } from 'react' | |||
type PaginationProps = { | |||
currentPage: number | |||
totalCount: number | |||
style?: 'black' | 'category' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1: style
is kind of reserved word. I this case variant
is the best naming.
2: Please revert this change as it's not sustainable. Whole DS is now being updated (a "black" pagination is what "category" was used for before).
We want to make a global change that will solve this for all components at once, I would like to avoid local minor changes to specific components.
Please revert this change. (And if we want to have black pagination, we can still use getCategoryColorLocalStyle({category: 'gray'})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅
next/components/molecules/SearchPageNew/useQueryBySearchOption.ts
Outdated
Show resolved
Hide resolved
next/components/molecules/SearchPageNew/GeneralSearchResults.tsx
Outdated
Show resolved
Hide resolved
{resultsCount.get(selectedKey) > 0 ? ( | ||
<p>{t('SearchPage.showingResults', { count: resultsCount.get(selectedKey) })}</p> | ||
{getResultsCountById(selectedKey) > 0 ? ( | ||
<p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use <Typography>
please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice job! I think we are ready to merge 🥳
Test build pipeline info 🚀 🔜 next |
Refactored main search page to follow recent design changes and allow searching for more content types.
Figma: https://www.figma.com/file/A9aoQH2FGhR1D14wvvk6FW/Mestsky-web-(bratislava.sk)?type=design&node-id=1223-1739&mode=design&t=lVTUvjy2sRM3DzOD-0
Satus 2024/01/05
Implemented:
Ready but hidden:
To be implemented:
Details can be found in #1072