Skip to content
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

549-refactor: Widget mentors #595

Merged
merged 13 commits into from
Oct 17, 2024
Merged

549-refactor: Widget mentors #595

merged 13 commits into from
Oct 17, 2024

Conversation

tanykos
Copy link
Collaborator

@tanykos tanykos commented Oct 9, 2024

What type of PR is this? (select all that apply)

  • πŸ• Feature
  • πŸ› Bug Fix
  • 🚧 Breaking Change
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ“ Documentation Update

Description

  1. mentors.scss
    • Refactor mentors.tsx's scss to scss modules
    • remove all parent selectors.
    • replace all element selectors by tag with class selector img -> .image
    • add items gap on mobile
  2. mentors.tsx
    • move mentors.test.tsx to ui folder
    • replace div's with HTML5 tags
  3. mentoring.test.tsx
    • replace become mentor hardcoded link with constant

Related Tickets & Documents

Added/updated tests?

  • πŸ‘Œ Yes
  • πŸ™…β€β™‚οΈ No, because they aren't needed
  • πŸ™‹β€β™‚οΈ No, because I need help

Summary by CodeRabbit

  • New Features

    • Introduced a new constant MENTORS_WANTED for improved URL management.
    • Enhanced the Mentors UI with a new SCSS file for better styling and responsiveness.
    • Added a dynamic registration link for the "Become a mentor" button.
  • Bug Fixes

    • Improved styling encapsulation by transitioning from traditional CSS to CSS modules.
  • Chores

    • Updated tests to reflect changes in the Mentors component and improved test structure.

@tanykos tanykos linked an issue Oct 9, 2024 that may be closed by this pull request
@github-actions github-actions bot changed the title 549-refactor: Widget Mentors 549-refactor: Widget mentors Oct 9, 2024
Copy link

github-actions bot commented Oct 9, 2024

Lighthouse Report:

  • Performance: 96 / 100
  • Accessibility: 100 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

Copy link

Lighthouse Report:

  • Performance: 80 / 100
  • Accessibility: 100 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

Copy link

Lighthouse Report:

  • Performance: 87 / 100
  • Accessibility: 100 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

@tanykos tanykos marked this pull request as ready for review October 10, 2024 11:01
@KristiBo
Copy link
Collaborator

Please specify which browser and version has this problem? I have everything fine on Chrome Version 129.0.6668.101.

Hmm... looked again and now it's ok

Copy link

Lighthouse Report:

  • Performance: 93 / 100
  • Accessibility: 100 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
src/widgets/mentors/ui/mentors.module.scss (1)

15-20: Approved: Mascot sizing for tablets.

The .sloth-mascot rule correctly sizes the mascot for tablets.

Consider adding styles for other screen sizes to ensure consistent appearance across all devices.

src/app/const/index.ts (1)

4-4: LGTM. Consider adding a comment.

The new constant is well-placed and follows the existing naming convention.

Add a brief comment explaining the purpose of MENTORS_WANTED:

 export const ANCHORS = {
   ABOUT_COMMUNITY: 'about-community',
   ABOUT_SCHOOL: 'about-school',
+  // Anchor for the section where mentors can register
   MENTORS_WANTED: 'mentors-wanted',
 };
πŸ“œ Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

πŸ“₯ Commits

Files that changed from the base of the PR and between ef9e059 and 6f1ea11.

πŸ“’ Files selected for processing (5)
  • src/app/const/index.ts (1 hunks)
  • src/widgets/mentors/ui/mentors.module.scss (1 hunks)
  • src/widgets/mentors/ui/mentors.test.tsx (1 hunks)
  • src/widgets/mentors/ui/mentors.tsx (2 hunks)
  • src/widgets/school-menu/ui/school-menu.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/widgets/mentors/ui/mentors.test.tsx
  • src/widgets/mentors/ui/mentors.tsx
🧰 Additional context used
πŸ”‡ Additional comments (4)
src/widgets/mentors/ui/mentors.module.scss (3)

1-6: Approved: Responsive layout structure.

The container and content styling provide a solid foundation for responsive design.

Also applies to: 22-26


7-13: Approved: Responsive width for info section.

The .mentors-info rule adapts well to different screen sizes.


1-27: Approved: Well-structured SCSS module.

The file demonstrates proper use of SCSS nesting and media queries, enhancing maintainability.

src/widgets/school-menu/ui/school-menu.tsx (1)

21-21: Good use of constant for URL.

Replacing hardcoded URL with ANCHORS.MENTORS_WANTED improves maintainability.

@Quiddlee Quiddlee requested review from SpaNb4 and ansivgit October 15, 2024 15:21
Copy link
Collaborator

@SpaNb4 SpaNb4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please pull the changes from the main branch to fix the failing test in CI/CD

Copy link

Lighthouse Report:

  • Performance: 98 / 100
  • Accessibility: 100 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

Copy link

Lighthouse Report:

  • Performance: 97 / 100
  • Accessibility: 100 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

@SpaNb4 SpaNb4 merged commit a81dfef into main Oct 17, 2024
4 checks passed
@SpaNb4 SpaNb4 deleted the refactor/549-widget-mentors branch October 17, 2024 10:27
@coderabbitai coderabbitai bot mentioned this pull request Oct 20, 2024
8 tasks
@coderabbitai coderabbitai bot mentioned this pull request Oct 28, 2024
8 tasks
@coderabbitai coderabbitai bot mentioned this pull request Dec 20, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FSD: Widget Mentors
6 participants