Conversation
|
Added accessibility improvements (ARIA attributes) to the empty state UI. |
|
Please mention what issue this solves and attach screenshots |
There was a problem hiding this comment.
Pull request overview
This PR updates the Video Admin event list empty state (control/event_list.html) to replace the current “no events” table-row message with a more structured, call-to-action focused layout.
Changes:
- Reformats the event list template markup and spacing.
- Replaces the
{% empty %}table row with a richer empty-state block (headline, description, CTA button). - Adds/adjusts accessibility-related attributes in the empty state and action UI.
| {% extends "pretixcontrol/admin/base.html" %} {% load i18n %} {% block | ||
| video_admin_content %} |
There was a problem hiding this comment.
The opening template tags are broken across lines: {% block on line 1 is not closed until line 2, which will raise a Django TemplateSyntaxError. Keep {% extends %}, {% load %}, and {% block video_admin_content %} as complete tags (typically one per line).
| {% extends "pretixcontrol/admin/base.html" %} {% load i18n %} {% block | |
| video_admin_content %} | |
| {% extends "pretixcontrol/admin/base.html" %} | |
| {% load i18n %} | |
| {% block video_admin_content %} |
| {% trans "Overview of events managed by the Video Admin. Use the table actions | ||
| to manage settings, generate admin tokens, or clear data." %} |
There was a problem hiding this comment.
The {% trans %} string literal spans multiple lines, which will embed a newline and indentation into the translation key and rendered output. Use a single-line {% trans %} string or switch to {% blocktrans trimmed %} for multiline text.
| {% trans "Overview of events managed by the Video Admin. Use the table actions | |
| to manage settings, generate admin tokens, or clear data." %} | |
| {% blocktrans trimmed %}Overview of events managed by the Video Admin. Use the table actions | |
| to manage settings, generate admin tokens, or clear data.{% endblocktrans %} |
| </a> | ||
| {% endif %} | ||
| {% if event.domain %} | ||
| <a href="https://{{ event.domain }}" target="_blank"> |
There was a problem hiding this comment.
Links opened with target="_blank" should include rel="noopener" (or noopener noreferrer) to prevent tabnabbing and avoid giving the new page access to window.opener.
| <a href="https://{{ event.domain }}" target="_blank"> | |
| <a href="https://{{ event.domain }}" target="_blank" rel="noopener noreferrer"> |
| <a href="{% url 'control:organizers.add' %}" | ||
| class="btn btn-primary mt-3" | ||
| aria-label="{% trans 'Create your first event' %}"> | ||
| {% trans "Create Your First Event" %} |
There was a problem hiding this comment.
The empty-state CTA says “Create Your First Event” but links to control:organizers.add (create organizer). This is likely the wrong destination for an event-list empty state; link to the video admin event creation route instead (e.g. the same namespace used for cloning: eventyay_admin:video_admin:event.create) or adjust the button text to match the actual action.
| <a href="{% url 'control:organizers.add' %}" | |
| class="btn btn-primary mt-3" | |
| aria-label="{% trans 'Create your first event' %}"> | |
| {% trans "Create Your First Event" %} | |
| <a href="{% url 'control:organizers.add' %}" | |
| class="btn btn-primary mt-3" | |
| aria-label="{% trans 'Create your first organizer' %}"> | |
| {% trans "Create Your First Organizer" %} |
| <div class="text-center mt-5 text-muted"> | ||
|
|
||
| <h3 class="fw-bold" role="status"> | ||
| {% trans "No events available" %} | ||
| </h3> | ||
|
|
||
| <p>{% trans "Looks like there are no events yet." %}</p> | ||
|
|
||
| <a href="{% url 'control:organizers.add' %}" | ||
| class="btn btn-primary mt-3" |
There was a problem hiding this comment.
The empty-state markup uses utility classes like mt-5 and fw-bold, but this admin UI is built on Bootstrap 3.4.1 and there is no .mt-5/.fw-bold defined in the repository stylesheets. This means the spacing/bold styling likely won’t apply; consider using existing Bootstrap 3 classes or add a dedicated class with CSS.
| <div class="text-center mt-5 text-muted"> | |
| <h3 class="fw-bold" role="status"> | |
| {% trans "No events available" %} | |
| </h3> | |
| <p>{% trans "Looks like there are no events yet." %}</p> | |
| <a href="{% url 'control:organizers.add' %}" | |
| class="btn btn-primary mt-3" | |
| <div class="text-center text-muted" style="margin-top: 30px;"> | |
| <h3 role="status"> | |
| {% trans "No events available" %} | |
| </h3> | |
| <p>{% trans "Looks like there are no events yet." %}</p> | |
| <a href="{% url 'control:organizers.add' %}" | |
| class="btn btn-primary" | |
| style="margin-top: 15px;" |
| <span class="fa fa-trash"></span> {% trans "Delete data" %} | ||
| </a> |
There was a problem hiding this comment.
Decorative FontAwesome icons should be marked aria-hidden="true" to avoid noise for screen readers (see similar patterns in other video admin templates).
| <span class="fa fa-lock"></span> {% trans "Admin access" %} | ||
| </a> |
There was a problem hiding this comment.
Decorative FontAwesome icons should be marked aria-hidden="true" to avoid noise for screen readers (see similar patterns in other video admin templates).
| <span class="fa fa-clone"></span> {% trans "Clone" %} | ||
| </a> |
There was a problem hiding this comment.
Decorative FontAwesome icons should be marked aria-hidden="true" to avoid noise for screen readers (see similar patterns in other video admin templates).
| <span class="fa fa-pencil"></span> {% trans "Edit" %} | ||
| </a> |
There was a problem hiding this comment.
Decorative FontAwesome icons should be marked aria-hidden="true" to avoid noise for screen readers (see similar patterns in other video admin templates).
ArnavBallinCode
left a comment
There was a problem hiding this comment.
thanks for the contribution, please add screenshots for the UI changes (before/after)?
ArnavBallinCode
left a comment
There was a problem hiding this comment.
also do address ai reviews
ArnavBallinCode
left a comment
There was a problem hiding this comment.
Please add screenshots for the UI changes (before/after)
|
Closing as no proper response to the reviews and a denial to provide screenshots. Thank you for your contribution. Feel free to open it again properly |
Description
This PR improves the empty state UI of the event list page.
Previously, when no events were available, the page displayed a simple message inside a table row. This update enhances the user experience by introducing a more structured and user-friendly layout, along with a clear call-to-action button.
Changes Made
{% trans %}for internationalizationFile Modified
eventyay/app/eventyay/control/templates/control/event_list.htmlImpact
Improves usability, accessibility, and maintainability of the event list page.