551: Fix Leaderboard Card Ordering On Mobile View#902
551: Fix Leaderboard Card Ordering On Mobile View#902isabellalam12 wants to merge 3 commits intomainfrom
Conversation
Available PR Commands
See: https://github.com/tahminator/codebloom/wiki/CI-Commands |
Title551: Fix Leaderboard Card Ordering On Mobile View PR TypeBug fix Description
Diagram Walkthroughflowchart LR
A[Dashboard Page] --> B{Is `smallPhone`?};
B -- Yes --> C[Problem of the Day then Leaderboard];
B -- No --> D[Leaderboard then Problem of the Day];
C --> E[Render Recent Submissions];
D --> E;
|
| Relevant files | |||||
|---|---|---|---|---|---|
| Enhancement |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Title551: Fix Leaderboard Card Ordering On Mobile View PR TypeEnhancement Description
Diagram Walkthroughflowchart LR
A[Dashboard Page] --> B{Is smallPhone?};
B -- Yes --> C[ProblemOfTheDay then DashboardLeaderboard];
B -- No --> D[DashboardLeaderboard then ProblemOfTheDay];
|
| Relevant files | |||||
|---|---|---|---|---|---|
| Enhancement |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Title551: Fix Leaderboard Card Ordering On Mobile View PR TypeBug fix, Enhancement Description
Diagram Walkthroughflowchart LR
Dashboard["Dashboard Page"] --> ConditionalOrder{"Conditional Component Order"};
ConditionalOrder -- "smallPhone = true" --> POTD_Leaderboard["Problem of the Day, then Leaderboard"];
ConditionalOrder -- "smallPhone = false" --> Leaderboard_POTD["Leaderboard, then Problem of the Day"];
|
| Relevant files | |||
|---|---|---|---|
| Enhancement |
| ||
| Responsive design |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Title551: Fix Leaderboard Card Ordering On Mobile View PR TypeEnhancement Description
Diagram Walkthroughflowchart LR
A[DashboardPage] --> B{Is smallPhone?};
B -- Yes --> C[Order: POTD, Leaderboard];
B -- No --> D[Order: Leaderboard, POTD];
C --> E[Render Cards];
D --> E;
|
| Relevant files | |||
|---|---|---|---|
| Enhancement |
| ||
| Styling |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
| {(() => { | ||
| const leaderboardCard = ( | ||
| <Flex direction={"column"} flex={1}> | ||
| <DashboardLeaderboard | ||
| userId={data.user.id} | ||
| userTags={data.user.tags} | ||
| /> | ||
| </Flex> | ||
| ); | ||
| const problemOfTheDayCard = ( | ||
| <Flex direction={"column"} flex={1}> | ||
| <ProblemOfTheDay /> | ||
| </Flex> | ||
| ); | ||
| const orderedComponents = | ||
| smallPhone ? | ||
| [problemOfTheDayCard, leaderboardCard] | ||
| : [leaderboardCard, problemOfTheDayCard]; | ||
| return <>{orderedComponents}</>; | ||
| })()} |
There was a problem hiding this comment.
Suggestion: The use of an immediately invoked function expression (IIFE) to conditionally order two components adds unnecessary complexity. For this scenario, a direct ternary operator within the JSX provides a clearer and more concise way to achieve the desired conditional rendering. This improves readability and aligns with the principle of keeping JSX declarative. [general, importance: 7]
| {(() => { | |
| const leaderboardCard = ( | |
| <Flex direction={"column"} flex={1}> | |
| <DashboardLeaderboard | |
| userId={data.user.id} | |
| userTags={data.user.tags} | |
| /> | |
| </Flex> | |
| ); | |
| const problemOfTheDayCard = ( | |
| <Flex direction={"column"} flex={1}> | |
| <ProblemOfTheDay /> | |
| </Flex> | |
| ); | |
| const orderedComponents = | |
| smallPhone ? | |
| [problemOfTheDayCard, leaderboardCard] | |
| : [leaderboardCard, problemOfTheDayCard]; | |
| return <>{orderedComponents}</>; | |
| })()} | |
| {smallPhone ? ( | |
| <> | |
| <Flex direction={"column"} flex={1}> | |
| <ProblemOfTheDay /> | |
| </Flex> | |
| <Flex direction={"column"} flex={1}> | |
| <DashboardLeaderboard | |
| userId={data.user.id} | |
| userTags={data.user.tags} | |
| /> | |
| </Flex> | |
| </> | |
| ) : ( | |
| <> | |
| <Flex direction={"column"} flex={1}> | |
| <DashboardLeaderboard | |
| userId={data.user.id} | |
| userTags={data.user.tags} | |
| /> | |
| </Flex> | |
| <Flex direction={"column"} flex={1}> | |
| <ProblemOfTheDay /> | |
| </Flex> | |
| </> | |
| )} |
|
/deploy |
Title551: Fix Leaderboard Card Ordering On Mobile View PR TypeBug fix, Enhancement Description
Diagram Walkthroughflowchart LR
DashboardPage -- "Check if smallPhone" --> ConditionalRender[Conditional Render]
ConditionalRender -- "True (smallPhone)" --> POTD_Leaderboard[ProblemOfTheDay, DashboardLeaderboard]
ConditionalRender -- "False (larger screen)" --> Leaderboard_POTD[DashboardLeaderboard, ProblemOfTheDay]
POTD_Leaderboard --> DisplayCards[Display Ordered Cards]
Leaderboard_POTD --> DisplayCards
|
| Relevant files | |||
|---|---|---|---|
| Enhancement |
| ||
| Bug fix |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
|
/deploy |
Title551: Fix Leaderboard Card Ordering On Mobile View PR TypeBug fix, Enhancement Description
Diagram Walkthroughflowchart LR
A[DashboardPage] --> B{Is smallPhone?};
B -- Yes --> C[Render POTD then Leaderboard];
B -- No --> D[Render Leaderboard then POTD];
C --> E[ProblemOfTheDay.tsx];
D --> E;
E -- Adjusts mih prop --> F[CodebloomCard];
|
| Relevant files | |||
|---|---|---|---|
| Enhancement |
| ||
| Bug fix |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
| {(() => { | ||
| const leaderboardCard = ( | ||
| <Flex direction={"column"} flex={1}> | ||
| <DashboardLeaderboard | ||
| userId={data.user.id} | ||
| userTags={data.user.tags} | ||
| /> | ||
| </Flex> | ||
| ); | ||
| const problemOfTheDayCard = ( | ||
| <Flex direction={"column"} flex={1}> | ||
| <ProblemOfTheDay /> | ||
| </Flex> | ||
| ); | ||
| const orderedComponents = | ||
| smallPhone ? | ||
| [problemOfTheDayCard, leaderboardCard] | ||
| : [leaderboardCard, problemOfTheDayCard]; | ||
| return <>{orderedComponents}</>; | ||
| })()} |
There was a problem hiding this comment.
Suggestion: The use of an Immediately Invoked Function Expression (IIFE) for conditional rendering can reduce readability. Consider defining the component variables directly within the Flex component's scope and then using a more direct conditional rendering approach. This simplifies the render logic and aligns with common React patterns. [general, importance: 6]
| {(() => { | |
| const leaderboardCard = ( | |
| <Flex direction={"column"} flex={1}> | |
| <DashboardLeaderboard | |
| userId={data.user.id} | |
| userTags={data.user.tags} | |
| /> | |
| </Flex> | |
| ); | |
| const problemOfTheDayCard = ( | |
| <Flex direction={"column"} flex={1}> | |
| <ProblemOfTheDay /> | |
| </Flex> | |
| ); | |
| const orderedComponents = | |
| smallPhone ? | |
| [problemOfTheDayCard, leaderboardCard] | |
| : [leaderboardCard, problemOfTheDayCard]; | |
| return <>{orderedComponents}</>; | |
| })()} | |
| const leaderboardCard = ( | |
| <Flex direction={"column"} flex={1}> | |
| <DashboardLeaderboard | |
| userId={data.user.id} | |
| userTags={data.user.tags} | |
| /> | |
| </Flex> | |
| ); | |
| const problemOfTheDayCard = ( | |
| <Flex direction={"column"} flex={1}> | |
| <ProblemOfTheDay /> | |
| </Flex> | |
| ); | |
| {smallPhone ? ( | |
| <> | |
| {problemOfTheDayCard} | |
| {leaderboardCard} | |
| </> | |
| ) : ( | |
| <> | |
| {leaderboardCard} | |
| {problemOfTheDayCard} | |
| </> | |
| )} |
|
/deploy |
isabellalam12
assigned
arklian and
Arshadul-Monir and unassigned
arklian and
Arshadul-Monir
|
Failed to generate code suggestions for PR |
3 participants
551
Description of changes
Checklist before review
Screenshots
Dev
Staging