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

feat: add e-lab testimonials #51

Merged
merged 4 commits into from
Jul 30, 2023
Merged

feat: add e-lab testimonials #51

merged 4 commits into from
Jul 30, 2023

Conversation

timonschramm
Copy link
Contributor

Hi there,
this website should be used starting on 01.08.
The Links for the application are currently pointing to unfinished Tallyforms, but they will be finished before the 01.08.
Please also change the Link in the navbar to the correct site then.

Thank you so much.

P.S The navbar looks still odd on phones

@vercel
Copy link

vercel bot commented Jul 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
website-nextjs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 29, 2023 8:56pm

@timonschramm timonschramm changed the title Launch of the E-Lab site for 01.08 - feat: Launch of the E-Lab site for 01.08 Jul 27, 2023
@timonschramm timonschramm changed the title - feat: Launch of the E-Lab site for 01.08 feat: Launch of the E-Lab site for 01.08 Jul 27, 2023
@simon-hng
Copy link
Collaborator

simon-hng commented Jul 28, 2023

Overall looks good.

  • you can change the link in the navigation in this pr as well
  • can you attach screenshots and explain the problem with the nav in more detail?
  • we'll open a pr after this is merged for your public deployment

Here are some remaining points:

You're still failing some checks:

  • prettier: run yarn format
  • pr title does not conform to conventional commits

I think the hero text looked better with left align. Any other opinions on that? @Max-vS @MunzerDw

The testimonials look nice and work really well

@Max-vS
Copy link
Contributor

Max-vS commented Jul 28, 2023

The size of Florian Scherls picture in the Testimonials is not a perfect circle, causing some arrangement issues.
I would agree to Simon: Hero looks better when left aligned.
Overall nice work!

@timonschramm
Copy link
Contributor Author

IMG_9007

this is the problem with the navbar on mobile

@Max-vS
Copy link
Contributor

Max-vS commented Jul 28, 2023

I think the issue with the Header on mobile, can be resolved by adding a margin to the top, when a certain media width is reached. Can be easily done with Tailwinds responsive features.

@simon-hng
Copy link
Collaborator

simon-hng commented Jul 28, 2023

It's not really an issue with margins but instead that this navigation was never intended to have 7 top level links.

For smaller screens like that we need some overflow scrolling

…to text center again, on mobile it's still centered || formatted with yarn format
@timonschramm
Copy link
Contributor Author

  • pr title does not conform to conventional commits

But when the PR Title passes it should be fine, shouldn't it?

@timonschramm
Copy link
Contributor Author

I also cropped the photo from florian

@timonschramm
Copy link
Contributor Author

I also added a video instead of the photo

@simon-hng simon-hng changed the title feat: Launch of the E-Lab site for 01.08 feat: add e-lab testimonials Jul 30, 2023
@simon-hng simon-hng merged commit c34cd29 into develop Jul 30, 2023
6 checks passed
@simon-hng simon-hng deleted the add-testimonials branch July 30, 2023 11:12
MunzerDw pushed a commit that referenced this pull request Jul 31, 2023
* Ready to publish for Application Phase starting on 01.08 || Added Testimonials

* Ready to publish for Application Phase starting on 01.08 || Added Testimonials || formatted

* formatted wit prettier

* swapped the links from aielab.tum... to /e-lab || switched the title to text center again, on mobile it's still centered || formatted with yarn format

---------

Co-authored-by: timonschramm <timonschramm@t-online.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants