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.

贡献者指南