BlitzKraft/saythanks.io

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

開放

#352 建立於 2025年8月6日

 (2 則留言) (0 個反應) (1 位負責人)Python (166 個分叉)github user discovery
good first issue

倉庫指標

星標
 (1,290 顆星)
PR 合併指標
 (平均合併 4天 11小時) (30 天內合併 27 個 PR)

描述

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.

貢獻者指南