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

feature/faq #428

Merged
merged 17 commits into from
Dec 17, 2024
Merged

feature/faq #428

merged 17 commits into from
Dec 17, 2024

Conversation

noahk004
Copy link
Contributor

@noahk004 noahk004 commented Dec 6, 2024

Completed FAQ section according to Figma.

  • Responsive design
  • Smooth transitions
    image
    image
    image

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 6, 2024 •
edited
Loading

Deploy preview for irvinehacks-site-2025 ready!

Name IrvineHacks Site
Preview Visit Preview
Commit 97eddcf

IanWearsHat
IanWearsHat previously requested changes Dec 6, 2024
View reviewed changes
Copy link
Member

@IanWearsHat IanWearsHat left a comment

Choose a reason for hiding this comment

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

Thanks for working on this Noah, the functionality looks great! I left a few minor comments.

@noahk004 noahk004 requested a review from IanWearsHat December 7, 2024 00:15
@taesungh taesungh linked an issue Dec 7, 2024 that may be closed by this pull request
FAQs #389
Closed
waalbert
waalbert previously requested changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@waalbert waalbert left a comment

Choose a reason for hiding this comment

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

Awesome start Noah! Here are a few suggestions:

apps/site/src/app/(main)/(home)/sections/FAQ/FAQ.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionDesktop.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/components/ListItemButton.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/components/ListItemButton.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionDesktop.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionDesktop.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionMobile.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionMobile.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionDesktop.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionDesktop.tsx Outdated Show resolved Hide resolved
@noahk004 noahk004 dismissed waalbert’s stale review December 16, 2024 20:29

Changes complete

@noahk004
Copy link
Contributor Author

duration-300 on many of the components are required to make sure that switching between the different screens (i.e., page 1, page 2, and question focused) is smooth

waalbert
waalbert requested changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@waalbert waalbert left a comment •
edited
Loading

Choose a reason for hiding this comment

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

For accessibility readers, using hidden rather than opacity is better even if we have to sacrifice the cool transition effect, but due to time constraints, we can take a look at this later if needed.

apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionDesktop.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionDesktop.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionDesktop.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionDesktop.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionDesktop.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionMobile.tsx Outdated Show resolved Hide resolved
apps/site/src/app/(main)/(home)/sections/FAQ/FAQAccordionMobile.tsx Outdated Show resolved Hide resolved
@noahk004 noahk004 requested a review from waalbert December 17, 2024 03:52
waalbert
waalbert approved these changes Dec 17, 2024
View reviewed changes
Copy link
Contributor

@waalbert waalbert left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this huge task Noah!

@waalbert waalbert merged commit e8bf60e into main Dec 17, 2024
4 checks passed
@waalbert waalbert mentioned this pull request Dec 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waalbert waalbert waalbert approved these changes

@IanWearsHat IanWearsHat Awaiting requested review from IanWearsHat

@jotalis jotalis Awaiting requested review from jotalis

@pandyrew pandyrew Awaiting requested review from pandyrew

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FAQs
3 participants
@noahk004 @IanWearsHat @waalbert