Handle no webinar scheduled state#164
Merged
Merged
Conversation
Contributor
WalkthroughThis PR adds conditional rendering to the webinar page based on a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assets/js/webinar.js (1)
6-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSet a fallback status on fetch failures.
layouts/webinar/single.htmlnow gates both the registration view and the newsletter fallback onwebinar.status. In this code path, a failed request only logs, so any API error—or a payload withoutstatus—leaves the page blank instead of showing the fallback state.💡 Proposed fix
constructor(webinarId, lang, languageNames, data) { $.getJSON(`${API_BASE_URL}/connect/contact/webinar/${webinarId}`).done(d => { data.name = d.name; - data.status = d.status; + data.status = d.status ?? 'Unavailable'; data.language = languageNames[d.language]; data.dateStart = d.dateStart.replace(' ', 'T') + 'Z'; data.lead = d.metadata.lead[lang]; data.learnTitle = d.metadata.learn.title[lang]; data.learnItems = (d.metadata.learn.items).map(i => i[lang]); }).fail(xhr => { + data.status = 'Unavailable'; console.error('Fetching webinar data failed:', xhr.responseJSON?.message || xhr.statusText); }); }🤖 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 `@assets/js/webinar.js` around lines 6 - 15, Ensure the page always gets a fallback status when the API fails or returns no status: in assets/js/webinar.js update the $.getJSON done callback to set data.status = d.status || 'fallback' (so missing status falls back), and in the fail callback explicitly assign data.status = 'fallback' (in addition to logging) so layouts/webinar/single.html can show the registration/newsletter fallback reliably; touch the done/fail handlers around the existing data assignments and the webinarId/data variables.
🤖 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/webinar/single.html`:
- Line 10: The title prefix is hardcoded in the x-text expression ("'Webinar: '
+ webinar.name") which forces English; update the h1 rendering to use a
localized string or omit the prefix: replace the inline "'Webinar: ' +
webinar.name" expression in the element with a localization lookup (e.g., a i18n
key like t('webinar.prefix') concatenated with webinar.name) or simply use
webinar.name alone, modifying the x-text attribute on the <h1> element
accordingly so the prefix is translatable or removed.
- Around line 167-170: The newsletter email input (x-model="data.email",
x-ref="email") currently relies on placeholder-only text; add an accessible
label by inserting a <label> element associated with the input via an id
attribute (e.g., id="newsletter-email") or, if you must not change layout, add
an aria-label on the input (aria-label="{{ i18n
'hub_newsletter_email_placeholder' }}") to provide a programmatic name; ensure
the label/aria-label is localized using the same i18n key and keep the input's
required attribute and x-model/x-ref intact so existing JS (new
Newsletter(...).subscribe()) continues to work.
---
Outside diff comments:
In `@assets/js/webinar.js`:
- Around line 6-15: Ensure the page always gets a fallback status when the API
fails or returns no status: in assets/js/webinar.js update the $.getJSON done
callback to set data.status = d.status || 'fallback' (so missing status falls
back), and in the fail callback explicitly assign data.status = 'fallback' (in
addition to logging) so layouts/webinar/single.html can show the
registration/newsletter fallback reliably; touch the done/fail handlers around
the existing data assignments and the webinarId/data variables.
🪄 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: 45313fa0-4c71-4c1a-8bab-e6cbe9323040
📒 Files selected for processing (4)
assets/js/webinar.jsi18n/de.yamli18n/en.yamllayouts/webinar/single.html
Comment on lines
+167
to
+170
| <form x-show="!data.success" x-ref="form" class="max-w-xl mx-auto mb-4" @submit.prevent="new Newsletter($refs.form, data).subscribe(); $refs.captcha.reset()"> | ||
| <p class="font-p mb-4">{{ i18n "webinar_none_newsletter_instruction" }}</p> | ||
| <div class="flex justify-center items-center rounded-sm bg-gray-300 mb-4"> | ||
| <input x-model="data.email" x-ref="email" @blur="$refs.email.classList.add('show-invalid')" type="email" class="grow input-box rounded-r-none focus:border-secondary" placeholder="{{ i18n "hub_newsletter_email_placeholder" }}" required/> |
Contributor
There was a problem hiding this comment.
Add a real label for the newsletter email field.
The new fallback form relies on placeholder text only, which is a weak accessible name for the single required input. Please add a <label> or aria-label here.
💡 Proposed fix
<form x-show="!data.success" x-ref="form" class="max-w-xl mx-auto mb-4" `@submit.prevent`="new Newsletter($refs.form, data).subscribe(); $refs.captcha.reset()">
<p class="font-p mb-4">{{ i18n "webinar_none_newsletter_instruction" }}</p>
<div class="flex justify-center items-center rounded-sm bg-gray-300 mb-4">
- <input x-model="data.email" x-ref="email" `@blur`="$refs.email.classList.add('show-invalid')" type="email" class="grow input-box rounded-r-none focus:border-secondary" placeholder="{{ i18n "hub_newsletter_email_placeholder" }}" required/>
+ <label for="webinar-newsletter-email" class="sr-only">{{ i18n "hub_newsletter_email_placeholder" }}</label>
+ <input id="webinar-newsletter-email" x-model="data.email" x-ref="email" `@blur`="$refs.email.classList.add('show-invalid')" type="email" class="grow input-box rounded-r-none focus:border-secondary" placeholder="{{ i18n "hub_newsletter_email_placeholder" }}" required/>
<button :disabled="data.inProgress || !data.acceptTerms || captchaState == 'verifying'" type="submit" class="shrink-0 flex items-center gap-1 btn btn-primary rounded-l-none px-4" data-umami-event="webinar-newsletter-form">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <form x-show="!data.success" x-ref="form" class="max-w-xl mx-auto mb-4" @submit.prevent="new Newsletter($refs.form, data).subscribe(); $refs.captcha.reset()"> | |
| <p class="font-p mb-4">{{ i18n "webinar_none_newsletter_instruction" }}</p> | |
| <div class="flex justify-center items-center rounded-sm bg-gray-300 mb-4"> | |
| <input x-model="data.email" x-ref="email" @blur="$refs.email.classList.add('show-invalid')" type="email" class="grow input-box rounded-r-none focus:border-secondary" placeholder="{{ i18n "hub_newsletter_email_placeholder" }}" required/> | |
| <form x-show="!data.success" x-ref="form" class="max-w-xl mx-auto mb-4" `@submit.prevent`="new Newsletter($refs.form, data).subscribe(); $refs.captcha.reset()"> | |
| <p class="font-p mb-4">{{ i18n "webinar_none_newsletter_instruction" }}</p> | |
| <div class="flex justify-center items-center rounded-sm bg-gray-300 mb-4"> | |
| <label for="webinar-newsletter-email" class="sr-only">{{ i18n "hub_newsletter_email_placeholder" }}</label> | |
| <input id="webinar-newsletter-email" x-model="data.email" x-ref="email" `@blur`="$refs.email.classList.add('show-invalid')" type="email" class="grow input-box rounded-r-none focus:border-secondary" placeholder="{{ i18n "hub_newsletter_email_placeholder" }}" required/> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 170-170: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 170-170: Special characters must be escaped : [ > ].
(spec-char-escape)
🤖 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` around lines 167 - 170, The newsletter email
input (x-model="data.email", x-ref="email") currently relies on placeholder-only
text; add an accessible label by inserting a <label> element associated with the
input via an id attribute (e.g., id="newsletter-email") or, if you must not
change layout, add an aria-label on the input (aria-label="{{ i18n
'hub_newsletter_email_placeholder' }}") to provide a programmatic name; ensure
the label/aria-label is localized using the same i18n key and keep the input's
required attribute and x-model/x-ref intact so existing JS (new
Newsletter(...).subscribe()) continues to work.
tobihagemann
approved these changes
May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Display the 'No Webinar Screen' when the webinar status is not 'Planned', so that we do not always need to have an active webinar scheduled
hint: set hide whitespace during review.