Skip to content

refactor: merge game info into home#189

Merged
jdrueckert merged 21 commits intomasterfrom
refactor/merge-game-info-into-home
Jan 8, 2023
Merged

refactor: merge game info into home#189
jdrueckert merged 21 commits intomasterfrom
refactor/merge-game-info-into-home

Conversation

@jdrueckert
Copy link
Copy Markdown
Member

@jdrueckert jdrueckert commented Jan 7, 2023

  • refactor: add "About" contents to "Home" page
  • chore: remove "Game" page and "About" component
  • chore: merge about scss into home scss
  • chore: move HighlightBox comonent into Home component subfolder

Action items before merging

  • fix "Learn more" link (gatsby "Link" and "navigate" don't support in-page routing)

@jdrueckert jdrueckert added this to the Feature & Content Additions milestone Jan 7, 2023
@jdrueckert
Copy link
Copy Markdown
Member Author

fix "Learn more" link (gatsby "Link" and "navigate" don't support in-page routing)

this might be the point at which we add the react/router dep...

@jdrueckert jdrueckert marked this pull request as ready for review January 8, 2023 15:24
@jdrueckert jdrueckert requested a review from skaldarnar January 8, 2023 15:24
Comment thread src/components/Home/Index.jsx Outdated
className="font-weight-bold btn btn-lg btn-success home-btn"
>
<Link to="/game" className="link-about">
<Link to="#about" className="link-about">
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Looks like we don't need the gatsby-plugin-anchor-links after all, I just needed to put the id on the row instead of the section as the section didn't do anything with it - we could also consider adding logic to add the id to the section div such that basically any section can be linked to.
Also, Link in comparison to AnchorLink doesn't scroll smoothly, but that would also be something which - if we want it - can be achieved using a react plugin like smooth-scroll instead of with a gatsby plugin.

Comment thread src/components/Home/Index.jsx Outdated
</div>
</Section>

<Row id="about" className="justify-content-center">
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

One aspect I don't like so much is that when we link to this, the section caption is not visible. I believe this is due to the height of our header obscuring a part of the page not being considered when jumping to the anchor. Any idea that currently comes to my mind on how to fix it would be an ugly hack, so I'm inclined to accept it for now until we have a good idea on how to solve this.

@jdrueckert jdrueckert mentioned this pull request Jan 8, 2023
1 task
skaldarnar
skaldarnar previously approved these changes Jan 8, 2023
…title)

All elements with `ts-anchor` have a scroll margin defined to adjust for the fixed header. The `Section` automatically assigns this class to the heading element.
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jan 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jdrueckert jdrueckert merged commit b416968 into master Jan 8, 2023
@jdrueckert jdrueckert deleted the refactor/merge-game-info-into-home branch January 8, 2023 23:52
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.

2 participants