search

bitcoin-core / bitcoincore.org

Bitcoin Core project website
https://bitcoincore.org/
MIT License
608 stars 474 forks source link

Update page.scss - Add Sticky Header/Fix Alignment #895

Open 2140data opened 2 years ago

2140data commented 2 years ago

Introduce a Sticky Header and Fix NavBar/Logo Alignment with 3 small CSS edits

Results: Adds Sticky Header and also fixes alignment of NavBar so it aligns with the Site Logo

Steps Taken:

  1. Created Sticky Header: Added position:sticky; and top:0; to .navigation-wrapper on page.scss
  2. Fixed NavBar Alignment: Edited .nav on page.scss from padding-top:1.5em; to padding-top:1.2em;
  3. Tested and verified locally as working
katesalazar commented 2 years ago

I think I once raised this same complaint in other PR somewhere else, but ATM don't remember where.

I like padding-top 1.5 better when 'nav ul li.lang' is positioned under "ABOUT"...

Id NAK eventually, only if and when I can review properly with pics and everything.

On Tue, Aug 23, 2022 at 6:18 PM Ant @.***> wrote:

Introduce a Sticky Header and Fix NavBar/Logo Alignment with 3 small CSS edits

Results: Adds Sticky Header and also fixes alignment of NavBar so it aligns with the Site Logo

Steps Taken:

  1. Created Sticky Header: Added position:sticky; and top:0; to .navigation-wrapper on page.scss
  2. Fixed NavBar Alignment: Edited .nav on page.scss from padding-top:1.5em; to padding-top:1.2em;
  3. Tested and verified locally as working

You can view, comment on, or merge this pull request online at:

https://github.com/bitcoin-core/bitcoincore.org/pull/895 Commit Summary

File Changes

(1 file https://github.com/bitcoin-core/bitcoincore.org/pull/895/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/bitcoin-core/bitcoincore.org/pull/895, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMRS4WY3D37J25QTB4IG5QDV2T2WDANCNFSM57MDZVZA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

2140data commented 2 years ago

On desktop with padding-top: 1.2em :

header

On smaller width with padding-top: 1.2em :

header-smaller

Here's a look at the Sticky header:

header-sticky-2
mikeobank commented 2 years ago
2140data commented 2 years ago

Yes, position:sticky has wide support (except IE (of course)), but it has built-in graceful degradation... if sticky header is not supported, it just stays static in the source (works as it does currently).

I am not seeing it broken on mobile, but I do see the flagged z-index issue (exists independently of this PR and could be fixed on its own).

Here is what I see on mobile:

Screen Shot 2022-08-24 at 8 59 09 AM

I also have a script for a better mobile alignment:

Screen Shot 2022-08-24 at 9 08 28 AM
katesalazar commented 2 years ago

there "smaller" looks better with the current padding-top, more elegantly aligned with the rest of the line, don't you think?

On Tue, Aug 23, 2022 at 10:17 PM Ant @.***> wrote:

On desktop with padding-top: 1.5em :

[image: header] https://user-images.githubusercontent.com/72945059/186256427-67262ec5-ffac-4296-be7f-400e37b82e42.png

On smaller width with padding-top: 1.5em :

[image: header-smaller] https://user-images.githubusercontent.com/72945059/186256601-3926689c-8ad9-45e6-851e-8ebb6d57f018.png

Here's a look at the Sticky header:

[image: header-sticky] https://user-images.githubusercontent.com/72945059/186257383-a5c1d1a3-8da7-4058-9fcb-51142a202b09.png

— Reply to this email directly, view it on GitHub https://github.com/bitcoin-core/bitcoincore.org/pull/895#issuecomment-1224803358, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMRS4WYF7SCW3S4P2BHOIN3V2UWUNANCNFSM57MDZVZA . You are receiving this because you commented.Message ID: @.***>

2140data commented 2 years ago

there "smaller" looks better with the current padding-top, more elegantly aligned with the rest of the line, don't you think? … On Tue, Aug 23, 2022 at 10:17 PM Ant @.> wrote: On desktop with padding-top: 1.5em : [image: header] https://user-images.githubusercontent.com/72945059/186256427-67262ec5-ffac-4296-be7f-400e37b82e42.png On smaller width with padding-top: 1.5em : [image: header-smaller] https://user-images.githubusercontent.com/72945059/186256601-3926689c-8ad9-45e6-851e-8ebb6d57f018.png Here's a look at the Sticky header: [image: header-sticky] https://user-images.githubusercontent.com/72945059/186257383-a5c1d1a3-8da7-4058-9fcb-51142a202b09.png — Reply to this email directly, view it on GitHub <#895 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMRS4WYF7SCW3S4P2BHOIN3V2UWUNANCNFSM57MDZVZA . You are receiving this because you commented.Message ID: @.>

Yes, I agree it does align properly when screens are smaller and the NavBar wraps to the second line... but is that the default view? On desktop, the NavBar appears misaligned. We could make it a true responsive header, align the NavBar and Logo properly across all scenarios, and add a couple simple media queries to handle the display on all screensizes.

--

Mid-Comment Update... I just went to make a screenshot of the desktop view and realized the alignment issue is actually on the logo, not the NavBar... the NavBar is properly aligned in the middle of the Navigation Area, but the logo is closer to the top than the bottom. It looks like Site-Name has padding-top: 1.15em; but padding-top: 1.4em; or padding-top: 1.5em; provides true alignment. Could use a media query to keep the alignment true at all breakpoints...

Screen Shot 2022-08-24 at 1 57 10 PM

.

mikeobank commented 2 years ago

I am not seeing it broken on mobile, but I do see the flagged z-index issue (exists independently of this PR and could be fixed on its own).

mikeobank commented 2 years ago
  • I do agree with fixing the z-index issues on all the other elements in a different PR. That would be cleaner than adding an arbitrarily large z-index to the header. That PR would be blocking getting this PR merged though.

It's probably not a z-index issue, but position: relative; instead. Something like this can fix it on the download page: https://github.com/bitcoin-core/bitcoincore.org/compare/master...mikeobank:bitcoincore.org:remove-image-position-relative. Not thoroughly tested.

2140data commented 2 years ago
  • I do agree with fixing the z-index issues on all the other elements in a different PR. That would be cleaner than adding an arbitrarily large z-index to the header. That PR would be blocking getting this PR merged though.

It's probably not a z-index issue, but position: relative; instead. Something like this can fix it on the download page: master...mikeobank:bitcoincore.org:remove-image-position-relative. Not thoroughly tested.

Nice! Removing position: relative; from the download images and magnet link would be a good fix. On the other hand, since it is truly a position issue and not a z-index issue, then .navigation-wrapper can be set to z-index: 1; or something low. Preliminary tests worked well with z-index:1.

mikeobank commented 2 years ago

.navigation-wrapper can be set to z-index: 1;

I agree

katesalazar commented 2 years ago

yes, that is the default view

On Wed, Aug 24, 2022 at 9:05 PM Ant @.***> wrote:

there "smaller" looks better with the current padding-top, more elegantly aligned with the rest of the line, don't you think? … <#m-5815734364623598347> On Tue, Aug 23, 2022 at 10:17 PM Ant @.> wrote: On desktop with padding-top: 1.5em : [image: header] https://user-images.githubusercontent.com/72945059/186256427-67262ec5-ffac-4296-be7f-400e37b82e42.png https://user-images.githubusercontent.com/72945059/186256427-67262ec5-ffac-4296-be7f-400e37b82e42.png On smaller width with padding-top: 1.5em : [image: header-smaller] https://user-images.githubusercontent.com/72945059/186256601-3926689c-8ad9-45e6-851e-8ebb6d57f018.png https://user-images.githubusercontent.com/72945059/186256601-3926689c-8ad9-45e6-851e-8ebb6d57f018.png Here's a look at the Sticky header: [image: header-sticky] https://user-images.githubusercontent.com/72945059/186257383-a5c1d1a3-8da7-4058-9fcb-51142a202b09.png https://user-images.githubusercontent.com/72945059/186257383-a5c1d1a3-8da7-4058-9fcb-51142a202b09.png — Reply to this email directly, view it on GitHub <#895 (comment) https://github.com/bitcoin-core/bitcoincore.org/pull/895#issuecomment-1224803358>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMRS4WYF7SCW3S4P2BHOIN3V2UWUNANCNFSM57MDZVZA https://github.com/notifications/unsubscribe-auth/AMRS4WYF7SCW3S4P2BHOIN3V2UWUNANCNFSM57MDZVZA . You are receiving this because you commented.Message ID: @.>

Yes, I agree it does align properly when screens are smaller and the NavBar wraps to the second line... but is that the default view? On desktop, the NavBar appears misaligned. We could make it a true responsive header, align the NavBar and Logo properly across all scenarios, and add a couple simple media queries to handle the display on all screensizes.

--

Mid-Comment Update... I just went to make a screenshot of the desktop view and realized the alignment issue is actually on the logo, not the NavBar... the NavBar is properly aligned in the middle of the Navigation Area, but the logo is closer to the top than the bottom. It looks like Site-Name has padding-top: 1.15em; but padding-top: 1.4em; or padding-top: 1.5em; provides true alignment. Could use a media query to keep the alignment true at all breakpoints...

[image: Screen Shot 2022-08-24 at 1 57 10 PM] https://user-images.githubusercontent.com/72945059/186502117-11cafe5b-aac1-43b3-9464-c1679460dbc8.png .

— Reply to this email directly, view it on GitHub https://github.com/bitcoin-core/bitcoincore.org/pull/895#issuecomment-1226126662, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMRS4WZAFVVC5J5YDUUXGY3V2ZW67ANCNFSM57MDZVZA . You are receiving this because you commented.Message ID: @.***>