Skip to content

Add support for dynamic setting of attributes from context based on #151#152

Open
maximbelyayev wants to merge 3 commits intojazzband:masterfrom
maximbelyayev:master
Open

Add support for dynamic setting of attributes from context based on #151#152
maximbelyayev wants to merge 3 commits intojazzband:masterfrom
maximbelyayev:master

Conversation

@maximbelyayev
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for dynamically setting widget attributes from the rendering context based on #151.

  • Introduces a mechanism to merge a dynamically resolved dictionary into the attributes via the special key "attr_dict".
  • Refactors the handling of boolean and "type" attribute values during rendering.
Files not reviewed (1)
  • README.rst: Language not supported

if v:
if isinstance(v, bool):
bounded_field = set_attr(bounded_field, f"{k}")
if k == "type":
Copy link

Copilot AI Apr 25, 2025

Choose a reason for hiding this comment

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

The current conditional structure may result in duplicate attribute assignments for non-'type' keys with boolean values. Consider restructuring the conditionals using if-elif-else to ensure that each attribute is processed only once.

Suggested change
if k == "type":
elif k == "type":

Copilot uses AI. Check for mistakes.
Comment thread README.rst

.. code-block:: html+django

context_dict_var = {"type":"text", id="my_username_id", placeholder="Login"}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this correct? If this is a Python dict then shouldn't it be:

Suggested change
context_dict_var = {"type":"text", id="my_username_id", placeholder="Login"}
context_dict_var = {"type":"text", "id": "my_username_id", "placeholder: "Login"}

@aleksihakli
Copy link
Copy Markdown
Member

@maximbelyayev sorry for the delay in the review, would you happen to have the opportunity to look into the review notes?

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.

3 participants