-
-
Notifications
You must be signed in to change notification settings - Fork 11
DRAFT: 883-feat: Make swipe gallery for merch card #889
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
base: main
Are you sure you want to change the base?
Changes from all commits
e993b26
2e66950
9eafcbe
109650d
0647f7b
bacb488
ed4c779
ef8479a
93d69c4
f604ceb
59b2777
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ | |
| "react-double-marquee": "^1.1.0", | ||
| "react-markdown": "^10.1.0", | ||
| "react-responsive-carousel": "^3.2.23", | ||
| "react-swipeable": "^7.0.2", | ||
| "react-youtube": "^10.1.0", | ||
| "rehype-autolink-headings": "^7.1.0", | ||
| "rehype-slug": "^6.0.0", | ||
|
|
@@ -46,7 +47,8 @@ | |
| "remark-rehype": "^11.1.2", | ||
| "remark-remove-comments": "^1.1.1", | ||
| "remark-toc": "^9.0.0", | ||
| "swiper": "^11.2.6" | ||
| "swiper": "^11.2.6", | ||
| "uuid": "^11.1.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@eslint/js": "^9.25.1", | ||
|
|
@@ -64,6 +66,7 @@ | |
| "@types/node": "^22.15.3", | ||
| "@types/react": "19.1.2", | ||
| "@types/react-dom": "19.1.3", | ||
| "@types/uuid": "^10.0.0", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This library was used in the 786 branch but it has now been removed. Try pulling the changes from branch 786 again, so that you have the current version and don't add unnecessary dependencies |
||
| "@vitejs/plugin-react": "^4.4.1", | ||
| "@vitest/coverage-v8": "^3.1.2", | ||
| "@vitest/eslint-plugin": "^1.1.44", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import { Metadata } from 'next'; | ||
|
|
||
| import { Merch } from '@/views/merch/merch'; | ||
|
|
||
| export async function generateMetadata(): Promise<Metadata> { | ||
| const title = 'Merch Β· The Rolling Scopes School'; | ||
|
|
||
| return { title }; | ||
| } | ||
|
|
||
| export default function CommunityRoute() { | ||
| return <Merch />; | ||
YulikK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { MerchResponse } from '../types'; | ||
| import { ApiServices } from '@/shared/types'; | ||
|
|
||
| export class MerchApi { | ||
| constructor(private readonly services: ApiServices) {} | ||
|
|
||
| public queryMerchCatalog() { | ||
| return this.services.rest.get<MerchResponse>(`merch/filelist.json`); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { v4 as uuidv4 } from 'uuid'; | ||
|
|
||
| import { ApiMerchItem, ApiMerchItemAdapt, MerchProduct, MerchResponse } from '../types'; | ||
|
|
||
| export const transformMerchCatalog = (data: MerchResponse): MerchProduct[] => { | ||
| const products: MerchProduct[] = []; | ||
| const baseUrl = process.env.API_BASE_URL; | ||
| const processCategory = (category: ApiMerchItemAdapt, parentTags: string[]) => { | ||
| for (const [key, value] of Object.entries(category)) { | ||
| if (isApiMerchItem(value)) { | ||
| products.push({ | ||
| id: uuidv4(), | ||
| name: key, | ||
| title: value.name, | ||
| preview: value.preview.map((path) => `${baseUrl}/${path}`), | ||
| download: value.download.map((path) => `${baseUrl}/${path}`), | ||
| tags: parentTags, | ||
| }); | ||
| } else { | ||
| processCategory(value, [...parentTags, key]); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| for (const [categoryName, categoryData] of Object.entries(data)) { | ||
| processCategory(categoryData, [categoryName]); | ||
| } | ||
|
|
||
| return products; | ||
| }; | ||
|
Comment on lines
+5
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion Add error handling and improve URL construction. The transformation function works well for flattening the nested catalog structure, but lacks error handling and proper URL construction. Consider these improvements:
|
||
|
|
||
| const isApiMerchItem = (item: unknown): item is ApiMerchItem => { | ||
| return Boolean( | ||
| item && typeof item === 'object' && 'name' in item && 'preview' in item && 'download' in item, | ||
| ); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| export type { MerchProduct } from './types'; | ||
| export { MerchCard } from './ui/merch-card/merch-card'; | ||
| export { merchStore } from './model/store'; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,18 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { transformMerchCatalog } from '../helpers/transform-merch-catalog'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { api } from '@/shared/api/api'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| class MerchStore { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| public loadMerchCatalog = async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const res = await api.merch.queryMerchCatalog(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| if (res.isSuccess) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return transformMerchCatalog(res.result); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error(e); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+5
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion Improve error handling in loadMerchCatalog method. Currently, the method returns undefined when an error occurs, which could lead to confusing UI states. Consider returning an empty array or error state that components can properly handle. public loadMerchCatalog = async () => {
try {
const res = await api.merch.queryMerchCatalog();
if (res.isSuccess) {
return transformMerchCatalog(res.result);
}
+ return []; // Return empty array if request was not successful
} catch (e) {
console.error(e);
+ return []; // Return empty array on error
}
};π Committable suggestion
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π§© Analysis chainπ Script executed: Length of output: 49 @ansivgit, I've created issue #975 to track the improvements for the |
||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| export const merchStore = new MerchStore(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| export type ApiMerchItem = { | ||
| name: string; | ||
| preview: string[]; | ||
| download: string[]; | ||
| }; | ||
|
|
||
| type ApiMerchCategory = { | ||
| [key: string]: ApiMerchItem; | ||
| }; | ||
|
|
||
| type ApiMerchData = { | ||
| [category: string]: ApiMerchCategory; | ||
| }; | ||
|
|
||
| export type ApiMerchItemAdapt = ApiMerchItem | ApiMerchCategory | ApiMerchData; | ||
| export type MerchResponse = { | ||
| [category: string]: ApiMerchData; | ||
| }; | ||
|
|
||
| export type MerchProduct = { | ||
| id: string; | ||
| name: string; | ||
| title: string; | ||
| preview: string[]; | ||
| download: string[]; | ||
| tags: string[]; | ||
| }; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,178 @@ | ||||||||||||||||||||||||
| .merch-card { | ||||||||||||||||||||||||
| position: relative; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| overflow: hidden; | ||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Overflow hidden may clip pagination dots |
||||||||||||||||||||||||
| display: flex; | ||||||||||||||||||||||||
| flex-direction: column; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| width: 100%; | ||||||||||||||||||||||||
| max-width: 320px; | ||||||||||||||||||||||||
| height: 100%; | ||||||||||||||||||||||||
| margin: 0 auto; | ||||||||||||||||||||||||
| border-radius: 5px; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| background-color: $color-white; | ||||||||||||||||||||||||
| box-shadow: 0 4px 12px 0 hsla(from $color-black h s l / $opacity-10); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| transition: box-shadow 0.3s ease; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| &:hover { | ||||||||||||||||||||||||
| box-shadow: | ||||||||||||||||||||||||
| 0 1px 5px hsla(from $color-black h s l / $opacity-40), | ||||||||||||||||||||||||
| 0 2px 8px -8px hsla(from $color-black h s l / $opacity-40); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .preview-wrap { | ||||||||||||||||||||||||
| position: relative; | ||||||||||||||||||||||||
| display: flex; | ||||||||||||||||||||||||
| height: 180px; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| &::after { | ||||||||||||||||||||||||
| pointer-events: none; | ||||||||||||||||||||||||
| content: ''; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| position: absolute; | ||||||||||||||||||||||||
| top: 0; | ||||||||||||||||||||||||
| left: 0; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| width: 100%; | ||||||||||||||||||||||||
| height: 100%; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| background: linear-gradient( | ||||||||||||||||||||||||
| to bottom, | ||||||||||||||||||||||||
| transparent 0%, | ||||||||||||||||||||||||
| hsla(from $color-black h s l / $opacity-10) 100% | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+31
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion Add explicit z-index to gradient overlay |
||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .info-wrap { | ||||||||||||||||||||||||
| display: flex; | ||||||||||||||||||||||||
| flex: 1 1; | ||||||||||||||||||||||||
| gap: 10px; | ||||||||||||||||||||||||
| align-items: center; | ||||||||||||||||||||||||
| justify-content: space-between; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| padding: 16px; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .preview { | ||||||||||||||||||||||||
| width: 100%; | ||||||||||||||||||||||||
| height: 100%; | ||||||||||||||||||||||||
| object-fit: contain; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .download { | ||||||||||||||||||||||||
| position: absolute; | ||||||||||||||||||||||||
| right: 10px; | ||||||||||||||||||||||||
| bottom: 10px; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| padding: 10px; | ||||||||||||||||||||||||
| border-radius: 100%; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| opacity: 0.8; | ||||||||||||||||||||||||
| background-color: $color-yellow; | ||||||||||||||||||||||||
| box-shadow: 0 4px 12px 0 hsla(from $color-black h s l / $opacity-10); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| &:hover { | ||||||||||||||||||||||||
| opacity: 1; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .download-img { | ||||||||||||||||||||||||
| width: 20px; | ||||||||||||||||||||||||
| height: 20px; | ||||||||||||||||||||||||
| border-radius: 5px; | ||||||||||||||||||||||||
| background-color: $color-yellow; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .swipe-btn { | ||||||||||||||||||||||||
| cursor: pointer; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| position: absolute; | ||||||||||||||||||||||||
| z-index: 2; | ||||||||||||||||||||||||
| top: 50%; | ||||||||||||||||||||||||
| transform: translateY(-50%); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| display: flex; | ||||||||||||||||||||||||
| align-items: center; | ||||||||||||||||||||||||
| justify-content: center; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| width: 36px; | ||||||||||||||||||||||||
| height: 36px; | ||||||||||||||||||||||||
| border: none; | ||||||||||||||||||||||||
| border-radius: 50%; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| font-size: 20px; | ||||||||||||||||||||||||
| color: $color-white; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| background-color: rgb(0 0 0 / 30%); | ||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please use this syntax: |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| transition: | ||||||||||||||||||||||||
| background-color 0.2s ease, | ||||||||||||||||||||||||
| transform 0.2s ease; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .arrow-img { | ||||||||||||||||||||||||
| filter: brightness(0) invert(1); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| &.is-prev { | ||||||||||||||||||||||||
| left: 8px; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| &.is-next { | ||||||||||||||||||||||||
| right: 8px; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+120
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets make this
Suggested change
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| &:hover { | ||||||||||||||||||||||||
| background-color: rgb(0 0 0 / 60%); | ||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the same, use hsla |
||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| &:disabled { | ||||||||||||||||||||||||
| cursor: default; | ||||||||||||||||||||||||
| opacity: 0.3; | ||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use $opacity-30 |
||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .swipe-dots { | ||||||||||||||||||||||||
| position: absolute; | ||||||||||||||||||||||||
| z-index: 2; | ||||||||||||||||||||||||
| bottom: -15px; | ||||||||||||||||||||||||
| left: 50%; | ||||||||||||||||||||||||
| transform: translateX(-50%); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| display: flex; | ||||||||||||||||||||||||
| gap: 6px; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .swipe-dot { | ||||||||||||||||||||||||
| width: 8px; | ||||||||||||||||||||||||
| height: 8px; | ||||||||||||||||||||||||
| border-radius: 50%; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| background-color: rgb(0 0 0 / 20%); | ||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use hsla |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| transition: background-color all 0.3s ease; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
Comment on lines
+153
to
+156
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix transition property syntax transition: background-color 0.3s ease;or transition: all 0.3s ease;Please correct this to ensure the active dot transitions smoothly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's good point, please fix it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No description provided. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A summary of the edits CodeRabbit can apply:
A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets): βοΈ src/entities/merch/ui/merch-card/merch-card.module.scssOpen src/entities/merch/ui/merch-card/merch-card.module.scss and navigate to the .swipe-dot block. Locate the line: Replace it exactly with: Save the file, restart your SCSS compiler or development server, and verify in the browser that the swipe dotsβ background-color now transitions smoothly without any syntax errors. A summary of the context that CodeRabbit is considering across the codebase:
|
||||||||||||||||||||||||
| &.active { | ||||||||||||||||||||||||
| background-color: $color-yellow; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .swipeable-area { | ||||||||||||||||||||||||
| touch-action: pan-y; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| position: relative; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| display: flex; | ||||||||||||||||||||||||
| flex: 1; | ||||||||||||||||||||||||
| align-items: center; | ||||||||||||||||||||||||
| justify-content: center; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @media (max-width: $mobile-landscape-width) { | ||||||||||||||||||||||||
| .swipe-btn { | ||||||||||||||||||||||||
| display: none; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
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.
Is there a reason we're using react-swipeable here instead of Swiper, given that Swiper is already listed in the dependencies?
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.
@gbogdanova good point, since we also have the following issue: #877