-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
522-refactor: Widget community #577
Conversation
β¦ed classNames & styles
Lighthouse Report:
|
Lighthouse Report:
|
Lighthouse Report:
|
π Walkthroughπ WalkthroughWalkthroughThe changes introduce a new Changes
Assessment against linked issues
Possibly related issues
Suggested reviewers
π Recent review detailsConfiguration used: .coderabbit.yaml π Files selected for processing (2)
π§ Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? πͺ§ TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 2
π§Ή Outside diff range and nitpick comments (7)
src/widgets/community-media/ui/community-media.module.scss (1)
13-19
: Consider adding a media query for small screens.The class handles laptop screens well, but might cause overflow on very small screens.
Add a media query for small screens:
@include media-mobile { width: 100%; max-width: 640px; }src/shared/ui/social-media-item/social-media-item.test.tsx (1)
19-19
: Component rendering updated correctly.Consider updating the test data identifier from 'social-media' to 'social-media-item' for consistency.
dev-data/community-media.data.tsx (1)
10-56
:communityGroups
constant looks good overall.The structure is consistent and appropriate for its purpose. Consider adding language codes to all relevant entries for consistency.
Example improvement:
- title: 'LinkedIn', + title: 'LinkedIn EN',src/widgets/community-media/ui/community-media.tsx (2)
7-7
: Consider using an absolute import path for 'data'.Using relative import paths can lead to maintenance issues as the project grows.
Replace the import with an absolute path:
-import { communityGroups } from 'data'; +import { communityGroups } from '@/data/community-groups';
31-31
: Enhance image alt text for better accessibility.The current alt text is good, but it could be more descriptive to improve accessibility.
Consider updating the alt text:
-<Image className={cx('sloth-mascot')} src={image} alt="A sloth mascot with a welcome" data-testid="welcome-sloth" /> +<Image className={cx('sloth-mascot')} src={image} alt="A friendly sloth mascot welcoming you to the RS Community" data-testid="welcome-sloth" />src/widgets/community-media/ui/community-media.test.tsx (1)
28-40
: Comprehensive content test with room for improvement.The test covers various aspects of the component's content and structure. Consider adding a test for the href attributes of social media links to ensure they're correctly set.
Add a test for social media link hrefs:
socialMediaItems.forEach((item, index) => { expect(item).toHaveAttribute('href', communityGroups[index].link); });src/widgets/pictures/ui/pictures.tsx (1)
Line range hint
1-90
: Partial refactoring detected.The changes in this file address the component renaming. However, other objectives mentioned in the PR, such as refactoring SCSS to modules and replacing div elements with HTML5 tags, are not reflected here. Consider completing these tasks for a comprehensive refactor.
π Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
π Files selected for processing (16)
- dev-data/community-media.data.tsx (1 hunks)
- dev-data/index.ts (1 hunks)
- src/pages/community.tsx (2 hunks)
- src/shared/ui/social-media-item/index.ts (1 hunks)
- src/shared/ui/social-media-item/social-media-item.test.tsx (2 hunks)
- src/shared/ui/social-media-item/social-media-item.tsx (2 hunks)
- src/shared/ui/social-media/index.ts (0 hunks)
- src/widgets/community-media/index.ts (1 hunks)
- src/widgets/community-media/ui/community-media.module.scss (1 hunks)
- src/widgets/community-media/ui/community-media.test.tsx (1 hunks)
- src/widgets/community-media/ui/community-media.tsx (1 hunks)
- src/widgets/community/community.test.tsx (0 hunks)
- src/widgets/community/index.ts (0 hunks)
- src/widgets/community/ui/community.scss (0 hunks)
- src/widgets/community/ui/community.tsx (0 hunks)
- src/widgets/pictures/ui/pictures.tsx (2 hunks)
π€ Files with no reviewable changes (5)
- src/shared/ui/social-media/index.ts
- src/widgets/community/community.test.tsx
- src/widgets/community/index.ts
- src/widgets/community/ui/community.scss
- src/widgets/community/ui/community.tsx
π Additional comments (19)
src/widgets/community-media/index.ts (1)
1-1
: Export looks good.The export statement is correct and aligns with the PR objectives of renaming components.
src/shared/ui/social-media-item/index.ts (1)
1-1
: Looks good.Correct export syntax. Facilitates clean imports.
src/shared/ui/social-media-item/social-media-item.tsx (2)
5-5
: Import path updated correctly.The import statement now reflects the component's new name.
15-15
: Component renamed as per PR objectives.The renaming improves clarity. Ensure all usages are updated.
Verify component usage:
src/widgets/community-media/ui/community-media.module.scss (2)
1-11
: Responsive layout implemented correctly.The
.community-media-content
class uses appropriate media queries for different screen sizes.
21-33
: Well-structured grid layout with proper responsiveness.The
.social-media-container
class implements a flexible grid layout that adapts well to different screen sizes.src/pages/community.tsx (2)
6-6
: Import statement updated correctly.The import for
CommunityMedia
has been added, replacing the previousCommunity
(imported asCommunitySection
) import. This change is consistent with the PR objectives.
31-31
: Component usage updated correctly.
<CommunityMedia />
now replaces<CommunitySection />
, maintaining consistency with the import change. This aligns with the PR's renaming objectives.src/shared/ui/social-media-item/social-media-item.test.tsx (3)
4-4
: Import statement updated correctly.
7-7
: Test suite description updated appropriately.
Line range hint
1-46
: Overall, changes reflect component renaming accurately.The test file has been updated consistently with the component renaming. Consider updating the test data identifier for full consistency.
dev-data/community-media.data.tsx (1)
1-8
: Imports look good.The necessary icons and types are imported correctly.
dev-data/index.ts (1)
17-17
: New export aligns with refactoring goals.The addition of
communityGroups
export is consistent and supports the community widget refactoring.src/widgets/community-media/ui/community-media.tsx (1)
9-11
: Styles setup looks good.The use of CSS modules and classNames binding is appropriate and follows best practices.
src/widgets/community-media/ui/community-media.test.tsx (3)
1-12
: Imports and variable declarations look good.The necessary testing libraries and components are imported, and variables for DOM elements are properly declared.
14-22
: Test setup is well-structured.The describe block and beforeEach hook are used effectively to set up the testing environment for the CommunityMedia component.
24-26
: Smoke test is implemented correctly.The test checks if the component renders without crashing, which is a good baseline test.
src/widgets/pictures/ui/pictures.tsx (2)
15-15
: Import change approved.The import statement now correctly reflects the renamed component and includes the props type.
86-86
: Component usage updated correctly.The
SocialMediaItem
component is now used consistently. Ensure that the prop types match the new component's expectations.
Lighthouse Report:
|
Lighthouse Report:
|
run visual now |
What type of PR is this? (select all that apply)
Description
community
tocommunity-media
.social-media
tosocial-media-item
.community-media.data.tsx
.HTML5
tags.community.tsx
's scss into scss modules.community.test.tsx
to the ui folder.Related Tickets & Documents
Screenshots, Recordings
Added/updated tests?
Summary by CodeRabbit
Release Notes
New Features
CommunityMedia
component showcasing social media links for the Rolling Scopes School.communityGroups
constant for easy access to social media details.Improvements
Pictures
component to utilize the newSocialMediaItem
for rendering links.Bug Fixes
Community
component and associated exports to streamline the codebase.Tests
CommunityMedia
component to ensure functionality and rendering accuracy.