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

Added Countdown for DMA on home page #87

Closed
wants to merge 2 commits into from

Conversation

gericci
Copy link
Collaborator

@gericci gericci commented Jan 14, 2024

Summary of Changes

  1. Fixed missing quotes on home page
  2. Create partial include "dma-countdown.njk"
  3. Added include to aside.njk
  4. Updated stylesheets with new DMA block
    -->

Copy link

netlify bot commented Jan 14, 2024

Deploy Preview for owa-production ready!

Name Link
🔨 Latest commit ac730a9
🔍 Latest deploy log https://app.netlify.com/sites/owa-production/deploys/65a4eb7431c2cd000857dc18
😎 Deploy Preview https://deploy-preview-87--owa-production.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@gericci
Copy link
Collaborator Author

gericci commented Jan 15, 2024

In order to put the countdown above the fold in small screens, I've duplicated the block (not possible to do otherwise in a robust manner). The problem is that the Javascript is called twice (not sure why). If somebody can take a look.

@rejhgadellaa
Copy link
Collaborator

Hmm ok the current implementation has two main issues:

  1. The dma-countdown.njk is included twice, which means the <script /> in there runs twice. The same script is declared again in home.html.

  2. The html/script uses getElementById, but that will always return a single element (I think the last one in the DOM). So that needs a rewrite.

I'll push a hotfix that addresses these issues but it's still far from perfect. If anyone has more time to improve it, that'd be great :) If not, we at least have a working version :)

@rejhgadellaa
Copy link
Collaborator

Ugh I can't write/update the PR, no time to figure out how to do it. I'll leave comments in code, it only required two changes to work.

@gericci gericci closed this Jan 15, 2024
@gericci gericci reopened this Jan 15, 2024
@rejhgadellaa
Copy link
Collaborator

Close in favor of #88

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