Skip to content

Re-Add webinar section#163

Merged
SailReal merged 5 commits into
developfrom
feature/webinar
May 20, 2026
Merged

Re-Add webinar section#163
SailReal merged 5 commits into
developfrom
feature/webinar

Conversation

@SailReal
Copy link
Copy Markdown
Member

No description provided.

@SailReal SailReal requested a review from tobihagemann May 19, 2026 07:52
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

Review Change Stack

Warning

Rate limit exceeded

@SailReal has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 30 minutes and 31 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5827328b-a470-41b2-bd87-9ababf9f3065

📥 Commits

Reviewing files that changed from the base of the PR and between cddb439 and 495e11c.

📒 Files selected for processing (2)
  • assets/js/webinar.js
  • layouts/partials/nav.html

Walkthrough

This PR refactors the form submission architecture and adds a new webinar registration feature. The HubContact class is replaced with a reusable ApiForm class that accepts a URL parameter for endpoint-agnostic form handling. Three existing contact form pages (become-a-partner, book-a-demo, contact-sales) are migrated to use the new ApiForm. A webinar feature is added with configuration settings, translations in English and German, a webinar banner integrated into the site layout, and a complete registration page that fetches webinar metadata, renders a localized form with Altcha captcha integration, and displays webinar details with Berlin timezone-formatted dates.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • cryptomator/cryptomator.github.io#129: Refactors contact form front-end logic by replacing HubContact with ApiForm across book-a-demo, contact-sales, and become-a-partner pages, directly overlapping with this PR's form migration.

Suggested reviewers

  • tobihagemann
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Re-Add webinar section' is vague and lacks specificity about the actual implementation, using a generic phrasing that doesn't convey the technical nature or scope of the changes. Consider a more specific title such as 'Add webinar registration page with form and banner' to better communicate the primary changes.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to evaluate whether there is any documented intent or context for the changes. Add a description that explains the purpose of the webinar section, the key changes made (refactored ApiForm, new templates, translations), and any relevant context.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/webinar

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@layouts/partials/nav.html`:
- Around line 7-11: The mobile menu max-height needs to account for the webinar
banner when .Scratch.Get "webinarBannerActive" is true; update the logic in
layouts/partials/nav.html to adjust the mobile menu's max-h (currently using
max-h-[calc(100vh-48px)]) when the banner is rendered (i18n "webinar_banner"
block) by subtracting the banner height (e.g. calc(100vh - 48px -
<bannerHeight>) or by toggling a CSS variable/class that sets --banner-height
and using max-height: calc(100vh - 48px - var(--banner-height))); ensure the
mobile menu element/class that uses max-h-[calc(100vh-48px)] is updated to use
the conditional value or variable when .Scratch.Get "webinarBannerActive" is
true so the nav viewport is not clipped.

In `@layouts/webinar/single.html`:
- Around line 173-176: The altcha module scripts are being accessed via
$altchaJs and $altchaWorkerJs and their templates reference .Data.Integrity even
though fingerprinting was not applied; update the resource pipeline to apply the
fingerprint function to these resources (or fingerprint after any intended
minification) so that .Data.Integrity is populated for the integrity attributes
on altcha.js and js/altcha/worker.js; specifically, locate the assignments to
$altchaJs and $altchaWorkerJs in the template and pipe them through |
fingerprint (or through your existing minify + fingerprint sequence) before
using .RelPermalink and .Data.Integrity.
- Line 6: The fetch chain that loads webinar metadata (fetch(API_BASE_URL +
'/connect/contact/webinar/' + submitData.webinarId)) lacks error handling and
assumes d.dateStart exists; update the chain to first check response.ok before
parsing JSON, wrap JSON parsing/use of d in a try/catch, and add a .catch(...)
to handle network/parse errors; inside the success path ensure you only call
d.dateStart.replace(...) when d.dateStart is a non-null string (or provide a
fallback), and on any error set feedbackData.errorMessage (and
feedbackData.inProgress = false) and optionally leave webinar with safe defaults
so the form/UI does not break; reference ApiForm, submitData.webinarId, webinar,
dateStart.replace and feedbackData when making these changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 93433305-03f1-4f73-b983-41046200c317

📥 Commits

Reviewing files that changed from the base of the PR and between 89d1381 and 51e8436.

📒 Files selected for processing (13)
  • assets/js/apiform.js
  • assets/js/webinar-datetime.js
  • config/_default/params.yaml
  • content/webinar.de.html
  • content/webinar.en.html
  • i18n/de.yaml
  • i18n/en.yaml
  • layouts/_default/baseof.html
  • layouts/become-a-partner/single.html
  • layouts/book-a-demo/single.html
  • layouts/contact-sales/single.html
  • layouts/partials/nav.html
  • layouts/webinar/single.html

Comment thread layouts/partials/nav.html
Comment thread layouts/webinar/single.html Outdated
Comment thread layouts/webinar/single.html
Comment thread assets/js/apiform.js
Comment thread content/webinar.de.html
Comment thread assets/js/apiform.js
Comment thread layouts/webinar/single.html Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
layouts/webinar/single.html (1)

6-6: ⚡ Quick win

Escape template values used inside JS literals in x-data/x-init.

On Line 6, interpolated values (webinarId, $lang, i18n strings) are inserted into JS single-quoted strings without JS escaping. A quote/backslash in translations or config can break Alpine initialization.

Proposed fix
-  <section x-data="{feedbackData: {success: false, inProgress: false, errorMessage: ''}, submitData: {webinarId: '{{ .Site.Params.webinar.webinarId }}', firstName: '', lastName: '', email: '', company: '', captcha: null, acceptNewsletter: false}, webinar: {name: '', language: '', dateStart: null, lead: '', learnTitle: '', learnItems: []}, acceptTerms: false, apiForm: null, captchaState: null}" x-init="apiForm = new ApiForm($refs.form, feedbackData, submitData, API_BASE_URL + '/connect/contact/register-webinar'); new Webinar(submitData.webinarId, '{{ $lang }}', {en: '{{ i18n "webinar_language_en" }}', de: '{{ i18n "webinar_language_de" }}'}, webinar)" class="container py-12">
+  <section x-data="{feedbackData: {success: false, inProgress: false, errorMessage: ''}, submitData: {webinarId: '{{ .Site.Params.webinar.webinarId | js }}', firstName: '', lastName: '', email: '', company: '', captcha: null, acceptNewsletter: false}, webinar: {name: '', language: '', dateStart: null, lead: '', learnTitle: '', learnItems: []}, acceptTerms: false, apiForm: null, captchaState: null}" x-init="apiForm = new ApiForm($refs.form, feedbackData, submitData, API_BASE_URL + '/connect/contact/register-webinar'); new Webinar(submitData.webinarId, '{{ $lang | js }}', {en: '{{ i18n "webinar_language_en" | js }}', de: '{{ i18n "webinar_language_de" | js }}'}, webinar)" class="container py-12">
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@layouts/webinar/single.html` at line 6, The Alpine x-data/x-init contains
unescaped template interpolations that are inserted into JS single-quoted
strings (submitData.webinarId, the second arg to new Webinar which uses $lang,
and the i18n strings passed as the language map), which can break JS if those
values contain quotes/backslashes; escape these values for JS by applying Hugo's
JS-safe escaping (e.g., the | js template function or jsonify) to each
interpolation used inside JS literals: wrap '{{ .Site.Params.webinar.webinarId
}}', '{{ $lang }}', '{{ i18n "webinar_language_en" }}', and '{{ i18n
"webinar_language_de" }}' with the JS-escaping helper so ApiForm and new Webinar
receive safe string literals.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@assets/js/webinar.js`:
- Line 9: Guard the dereference of d.dateStart before calling .replace() to
avoid runtime exceptions: in the code that sets data.dateStart (referencing
d.dateStart.replace), check that d.dateStart is a non-empty string (or otherwise
truthy) and only call replace when safe; otherwise set data.dateStart to a
sensible fallback (e.g., null, empty string, or a normalized ISO default) so the
webinar state initialization cannot crash if the API returns missing or
malformed dateStart.

---

Nitpick comments:
In `@layouts/webinar/single.html`:
- Line 6: The Alpine x-data/x-init contains unescaped template interpolations
that are inserted into JS single-quoted strings (submitData.webinarId, the
second arg to new Webinar which uses $lang, and the i18n strings passed as the
language map), which can break JS if those values contain quotes/backslashes;
escape these values for JS by applying Hugo's JS-safe escaping (e.g., the | js
template function or jsonify) to each interpolation used inside JS literals:
wrap '{{ .Site.Params.webinar.webinarId }}', '{{ $lang }}', '{{ i18n
"webinar_language_en" }}', and '{{ i18n "webinar_language_de" }}' with the
JS-escaping helper so ApiForm and new Webinar receive safe string literals.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f6737826-518f-487c-b389-48e25282e341

📥 Commits

Reviewing files that changed from the base of the PR and between 51e8436 and cddb439.

📒 Files selected for processing (4)
  • assets/js/webinar.js
  • content/webinar.de.html
  • content/webinar.en.html
  • layouts/webinar/single.html
✅ Files skipped from review due to trivial changes (1)
  • content/webinar.de.html

Comment thread assets/js/webinar.js
@SailReal SailReal requested a review from tobihagemann May 20, 2026 09:13
@SailReal SailReal merged commit 9a5e036 into develop May 20, 2026
5 checks passed
@SailReal SailReal deleted the feature/webinar branch May 20, 2026 09:46
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