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

BrickHack 8 Site - Hero Section #1253

Merged
merged 6 commits into from
Dec 20, 2021
Merged

BrickHack 8 Site - Hero Section #1253

merged 6 commits into from
Dec 20, 2021

Conversation

sjmiller7
Copy link
Contributor

Fixes #1247

Need to check with Design on breakpoints and handling mobile. This Ricky has a lot of spacing on him and I want to make sure he looks alright.

@sjmiller7 sjmiller7 added the BH8 Ohhhh yea! label Nov 28, 2021
@sjmiller7 sjmiller7 requested a review from peterkos November 28, 2021 05:47
@sjmiller7 sjmiller7 self-assigned this Nov 28, 2021
Copy link
Contributor

@peterkos peterkos left a comment

Choose a reason for hiding this comment

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

Great work!

Main comment is seeing if we can bump up the video resolution vs. bandwidth, and a couple of UX tweaks.

Is this how you expect it to look on a 16" screen? Also might be worth testing on the monitors in the club room.
CleanShot 2021-11-29 at 01 28 46@2x

@skyegallup skyegallup self-requested a review November 30, 2021 17:36
Copy link

@skyegallup skyegallup left a comment

Choose a reason for hiding this comment

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

One nit, one mobile issue commented in code.

Another general nit: Sun God Ricky leaves a trail of flaming text in his wake The landing video slightly clips the content at some viewport widths. I know this is a very hard problem to deal with though, and it'll probably need to get changed anyway once design team returns with a margin-less video file, so it's not a big complaint - feel free to ignore this.
image

@sjmiller7
Copy link
Contributor Author

The landing video slightly clips the content at some viewport widths.

Going to ask Design about changing this layout a bit.

signup form

This will be going away as I swap it out for login and signup buttons. Going to do that and fixes probably tomorrow

@sjmiller7
Copy link
Contributor Author

I don't feel great about the hero. Something is still a bit off in its transition to mobile but I can't tell how to fix it. I don't want the text to wrap on the content bit but I'm having trouble getting it to balance with the video. Might be something with flex grow/shrink I'm missing that could fix it? It also feels like the video is HUGE when I swap to mobile... I could cut down the width maybe and give it more space on the sides? Or trying to bring down the point where I swap it from row to column? Lmk what you all think

Leaving out login and register buttons in the navbar until the general pr.

@sjmiller7
Copy link
Contributor Author

Did some fixes based on critique from design team. At this point, I just want to merge it in so design can actually see any further changes at a preview link-- that will take out a lot of the struggle we are dealing with for blocking right now.

So as long as there's no code issues right now, I suggest we save design nits for main branch and go ahead and merge everything.

Copy link
Contributor

@peterkos peterkos left a comment

Choose a reason for hiding this comment

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

There's a lot of tweaks that need to be made (mobile bugs, video position, etc.) but if design needs a preview this can be merged in.

For the next PR, 1rem padding bottom isnt needed cause of the intrinsic margin of the text at that size:
CleanShot 2021-12-18 at 23 58 14

@sjmiller7
Copy link
Contributor Author

I know it's super buggy and ugly right now, I am not happy with it. I really just want design to look and have the final say, which is tough to do through screenshots and over discord with communication at the pace it currently is. Again, if we have no big code problems in this and the other prs I say we just merge. I can probably look at merging Monday evening (12/20)

@sjmiller7 sjmiller7 requested a review from skyegallup December 19, 2021 07:08
Copy link

@skyegallup skyegallup left a comment

Choose a reason for hiding this comment

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

👍 good divs

@sjmiller7
Copy link
Contributor Author

Don't know why I can't comment directly on this comment:

👍 video quality looks fine on my 1080p, 13.4" display

When the notif for that comment appeared in my inbox, it showed you were commenting on old code that still had the mailing form. Can you double check?

@skyegallup
Copy link

+1 video quality looks fine on my 1080p, 13.4" display

When the notif for that comment appeared in my inbox, it showed you were commenting on old code that still had the mailing form. Can you double check?

no, I commented on the latest version, I think
image

@peterkos
Copy link
Contributor

LGTM

@sjmiller7 sjmiller7 merged commit bd5b7a9 into bh-1246 Dec 20, 2021
@sjmiller7 sjmiller7 deleted the bh-1247 branch December 20, 2021 00:22
sjmiller7 added a commit that referenced this pull request Jan 2, 2022
* swapping files and start gutting bh7 copy

* more gutting of css

* style refactor

* logo update

* design fixes

* mobile navbar

* design fixes

* social cards being funky

* editing social card resolution

* adjusting social card

* favicon swap

* nav underline

* outline color fix

* remove login

* fixes

* comment fixes

* contact first attempt

* changing social links

* Revert "comment fixes"

This reverts commit 75f57c6.

* Revert "Revert "comment fixes""

This reverts commit 6c75a58.

* Revert "changing social links"

This reverts commit fef9a7b.

* Revert "contact first attempt"

This reverts commit d980e9a.

* BrickHack 8 Site - Hero Section (#1253)

* starting hero

* hero section

* swapping out loop file

* fixes

* design fixes

* spacing fix

* BrickHack 8 Site - About Section (#1254)

* inital about section

* card width

* fixes

* BrickHack 8 Site - FAQ Section (#1252)

* faq styling + ricky

* fixing mobile

* removing old padding class

* fixes

* fixing broken cards

* BrickHack 8 Site - Contact & Footer (#1256)

* contact section

* fixes

* newline eof

Co-authored-by: Skye Gallup <[email protected]>

* fixing aspect ratio code

Co-authored-by: Skye Gallup <[email protected]>

* design + js fixes

* design fixes

* trying to hide play button

* trying to hide play button 2

* trying to hide play button 3

* fixing faq ricky

* fixing mobile grayout opacity

* hiding mlh banner

* copyright date

* updating stats

* prepping for open applications

* content fixes for chris

* spelling error

Co-authored-by: Chris Baudouin, Jr. <[email protected]>

* fixing spelling error

* fixing faq padding

* hero content adjustments

* hero content addition

* centering hero on mobile

Co-authored-by: Skye Gallup <[email protected]>
Co-authored-by: Chris Baudouin, Jr. <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BH8 Ohhhh yea!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants