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.

Sign up for GitHub

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

Jump to bottom

Fixes #58 - Show doctors section on home page & Adding components to storybook #60

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fixes #58 - Show doctors section on home page & Adding components to storybook #60

wants to merge 3 commits into from

Conversation

rray524
Copy link
Collaborator

@rray524 rray524 commented Nov 4, 2024 •
edited
Loading

Checklist:

  • Done: The design will be similar to homy's Our Agents section
  • Done: Show following details for each profile:
  1. Picture
  2. Name
  3. Rating
  4. Specialisation

Resolves #58

How to Add the Doctor section?

<div className="doctor-section z-10">
        <Container className="pt-8 pb-4">
          <DoctorSectionWrapper />
        </Container>
 </div>

How to Get All components of Doctor Team section

sections > OurDoctors folder

Description

The "Doctor" section typically includes detailed profiles of medical professionals, covering their specialties, qualifications, experience, and availability. Key features often include booking options for consultations, reviews from patients, and contact information. This section aims to help patients make informed choices and streamline appointment scheduling.

Here, additionally storybook is added for below mentioned components for a better designer view:

  • Container
  • DoctorSlider
  • StarRating

Command run: npm run storybook

image
image

@rray524 rray524 requested a review from rt4914 November 4, 2024 00:57
rt4914
rt4914 requested changes Nov 4, 2024
View reviewed changes
Copy link
Owner

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

@rray524 Great work.
I have left some suggestions. Also feel free to shift storybook items to next PR but in that case udpate the PR title and description to Fixes part of #xx instead of Fixes #xx and Resolves #xx.

Feel free to reach out if there any doubts.

src/assets/img_01.webp Outdated
Copy link
Owner

Choose a reason for hiding this comment

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

Add all these images inside assets/temp folder so that once our project is ready we know that we can delete this temp folder.

Copy link
Collaborator Author

@rray524 rray524 Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Here, I have added the storybook inside this PR - changed title and description and images are shifted to under temp folder. Thank you so much for your kind review.

src/components/Paragraph.jsx Outdated
return (
<p
className={`text-2xl leading-relaxed text-gray-500 ${className} font-normal`}
>
Copy link
Owner

Choose a reason for hiding this comment

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

Convert this to single line

<p className={text-2xl leading-relaxed text-gray-500 ${className} font-normal}>

And add storybook item for this component.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Thanks

src/containers/Container.jsx
@@ -0,0 +1,20 @@
import PropTypes from "prop-types";

const Container = ({ children, className }) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Add story book item fro this component.

Nice work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, added storybook for this.

src/index.css Outdated
@@ -5,3 +5,47 @@
body {
font-family: "Open Sans", Arial, sans-serif;
}
.with-background::before {
Copy link
Owner

Choose a reason for hiding this comment

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

Create a separate slick.css file and import that only for the required component.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, thanks

src/sections/OurDoctors/DoctorSectionWrapper.jsx Outdated
import HeaderTitle from "./HeaderTitle";
import TeamSlider from "./TeamSlider";

const DoctorSectionWrapper = () => {
Copy link
Owner

Choose a reason for hiding this comment

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

Remove OurDoctors folder and keep this directly in sections.

Also renamed to TopDoctorsSection.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Solved it. thank you!

src/sections/OurDoctors/HeaderTitle.jsx Outdated
@@ -0,0 +1,14 @@
import HeaderText from "../../components/HeaderText";

const HeaderTitle = () => {
Copy link
Owner

Choose a reason for hiding this comment

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

This is not required, directly use HeaderText where required.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Solved it. thank you! HeaderText is being used directly now!

src/sections/OurDoctors/StarRating.jsx Outdated
import PropTypes from "prop-types";
import "@fortawesome/fontawesome-free/css/all.css";

const StarRating = ({ rating }) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Shift this to component folder and also add a storybook for this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved. thanks!

src/sections/OurDoctors/TeamSlider.jsx Outdated
},
];

const TeamSlider = () => {
Copy link
Owner

Choose a reason for hiding this comment

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

This should be doctor slider.

Also add this to containers and not sections
Also add storyboook item for this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved. thanks!

@rt4914 rt4914 assigned rray524 and unassigned rt4914 Nov 4, 2024
rt4914
rt4914 reviewed Nov 4, 2024
View reviewed changes
src/sections/OurDoctors/TeamSlider.jsx Outdated
],
};

return (
Copy link
Owner

Choose a reason for hiding this comment

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

This slider does not work on mobile screen. In that there is one item visible which is correct but there are only 2 dots, which means that dots/indicators are not responsive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved. thanks!

rt4914
rt4914 reviewed Nov 4, 2024
View reviewed changes
src/sections/OurDoctors/TeamSlider.jsx Outdated
</Slider>
);
};
TeamSlider.propTypes = {
Copy link
Owner

Choose a reason for hiding this comment

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

Add one blank line above this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved. thanks!

@rray524 rray524 changed the title Fixes #58 - Show doctors section on home page Fixes #58 - Show doctors section on home page & Adding components to storybook Nov 5, 2024
@rray524 rray524 assigned rt4914 and unassigned rray524 Nov 5, 2024
@rray524 rray524 requested a review from rt4914 November 5, 2024 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rt4914 rt4914 Awaiting requested review from rt4914

Requested changes must be addressed to merge this pull request.

Assignees

@rt4914 rt4914

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Show doctors section on home page
2 participants
@rray524 @rt4914