BlitzKraft/saythanks.io

Align with Flask and Jinja best practices - always prefer url_for and semantic HTML

Aberta

#352 aberto em 6 de ago. de 2025

 (2 comentários) (0 reação) (1 responsável)Python (166 forks)github user discovery
good first issue

Métricas do repositório

Stars
 (1.290 estrelas)
Métricas de merge de PR
 (Mesclagem média 4d 11h) (27 fundiu PRs em 30d)

Description

https://github.com/BlitzKraft/saythanks.io/blob/8768bb0f89bfced59006ceb67426085c8bce209c/saythanks/templates/submit_note.htm.j2#L39

1. Code Smells & Antipatterns

  • Complex Jinja Expression in action Attribute:
    The form's action attribute uses a nested Jinja template with {% if topic %}{{ topic|replace(' about ', '') }}{% else %}{% endif %}. This logic is hard to read, can be error-prone, and mixes template logic with URL construction.
  • Relative URLs:
    The form action uses relative paths (./{{ user }}/submit/...), which can be fragile and less clear than using Flask’s url_for.
  • HTML Attribute Formatting:
    The action attribute construction might result in malformed URLs if topic contains spaces or special characters not properly escaped.
  • Semantic HTML:
    Table layout for form controls is outdated and makes accessibility harder. Modern HTML prefers using <div>s and semantic form tags.
  • Unused Else Block:
    The {% else %}{% endif %} in the action attribute does nothing and can be omitted for clarity.

2. Concrete Recommendations

a) Use Flask’s url_for for Action URLs

Instead of manually constructing the path, use Flask’s url_for and pass parameters:

<form id="thankyou-note-form" class="form-horizontal"
      action="{{ url_for('submit_note', user=user, topic=topic|replace(' about ', '') if topic else None) }}"
      method="post">

This assumes you have a Flask route named submit_note accepting user and optional topic.

b) Simplify Jinja Logic

If you must build the path manually, break up logic for clarity:

{% set topic_str = topic|replace(' about ', '') if topic else '' %}
<form id="thankyou-note-form" class="form-horizontal"
      action="./{{ user }}/submit/{{ topic_str }}"
      method="post">

c) Use Semantic HTML for Form Layout

Replace <table> with semantic tags for better accessibility:

<div class="form-row">
  <label for="fontstyle">Font Style:</label>
  <select name="Font Style" id="fontstyle" onchange="fontforinbox(this);">
    ...
  </select>
</div>
<div class="form-row">
  <label>Content Type:</label>
  <input type="radio" name="content-type" id="option1" value="markdown" checked> Plain Text / Markdown
  <input type="radio" name="content-type" id="option2" value="html"> HTML
</div>

d) Sanitize and Escape URL Parameters

If you use parameters in URLs, ensure they are URL-encoded. In Jinja, use the |urlencode filter:

action="./{{ user|urlencode }}/submit/{{ topic_str|urlencode }}"

3. Best Practices in Jinja & Flask

  • Prefer url_for for building URLs.
  • Minimize logic inside template attributes.
  • Use semantic HTML for forms.
  • Always escape user-provided data in URLs.

Example Refactored Snippet

{% set topic_str = topic|replace(' about ', '') if topic else '' %}
<form id="thankyou-note-form" class="form-horizontal"
      action="{{ url_for('submit_note', user=user|urlencode, topic=topic_str|urlencode) }}"
      method="post">

Summary:
These changes improve readability, maintainability, and robustness of your form action and layout. They also align with Flask and Jinja best practices. If your routing doesn’t match, adjust accordingly, but always prefer url_for and semantic HTML.

Guia do colaborador