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

Admin missing translations and tiny improvements. #2023

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

amazingphilippe
Copy link
Contributor

Summary | Résumé

This addresses cds-snc/notification-planning#998

  • Adds or calls translation keys where missing
  • Adds translations where missing
  • Some style tweaks on the live service table to improve readability.

Test instructions | Instructions pour tester la modification

Sequential steps (1., 2., 3., ...) that describe how to test this change. This
will help a developer test things out without too much detective work. Also,
include any environmental setup steps that aren't in the normal README steps
and/or any time-based elements that this requires.


Étapes consécutives (1., 2., 3., …) qui décrivent la façon de tester la
modification. Elles aideront les développeurs à faire des tests sans avoir à
jouer au détective. Veuillez aussi inclure toutes les étapes de configuration
de l’environnement qui ne font pas partie des étapes normales dans le fichier
README et tout élément temporel requis.

Copy link

<p class="mb-0 text-base text-gray-800">{{ message }}</p>
{% macro empty_list(heading, message=None, img="emptyFlower", link=None, linkText="", attributes="") %}
<div
class="flex items-center border-dashed border-1 border-gray-400 p-8 sm:bg-{{img}} sm:bg-right bg-empty-state min-h-emptyState"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've only added a min-height. The rest is auto formatting.

<td class="table-empty-message p-6" colspan="10">
{{ empty_message }}
<td class="table-empty-message h-10" colspan="10">
{{ empty_list(heading=empty_message) }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not use the empty component!

{% macro row_heading(cell_width=None) -%}
<th class="table-field {{ cell_width if cell_width}}" scope="row">
{% macro row_heading(cell_width=None, rowspan="1") -%}
<th class="table-field {{ cell_width if cell_width}}" scope="row" rowspan="{{ rowspan }}">
Copy link
Contributor Author

@amazingphilippe amazingphilippe Dec 20, 2024

Choose a reason for hiding this comment

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

This lets us create <th> that span many rows, like we have on the admin panel for each service usage. Defaults to 1

<li>
<a href="{{ url_for('.organisation_dashboard', org_id=current_org.id) }}" {{ org_navigation.is_selected('dashboard')
}}>
{{ _("Usage") }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks worse than it is, I simply added the _() calls and reformated the html.

@@ -103,7 +103,7 @@
{{ nav_menu_item(url=url_for('main.contact', service_id=current_service.id),localised_txt=_('Contact us'), is_active=header_navigation.is_selected('contact')) }}
{% endif %}
{% else %} {# current_user.platform_admin #}
{% if not platform_admin_view_ind %}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes the empty main navigation in platform admin. ✨ No more layout shift ✨ 🎉

@@ -113,9 +113,10 @@
{{ nav_menu_item(url=url_for('main.service_settings', service_id=current_service.id),localised_txt=_('Settings'), is_active=header_navigation.is_selected('settings')) }}
{% else %} {# not current_user.has_permissions, i.e. services not in context #}
{{ nav_menu_item(url=url_for('main.choose_account'),localised_txt=_('Your services'),css_classes='pl-0',id_key='choose_account', is_active=header_navigation.is_selected('choose_account')) }}
{{ nav_menu_item(url=url_for('main.live_services', service_id=current_service.id),localised_txt=_('Admin panel')) }}
{{ nav_menu_item(url=url_for('main.live_services', service_id=current_service.id),localised_txt=_('Admin panel'),
is_active=header_navigation) }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just adding the aria-current

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.

1 participant