Skip to content

Add MacroComponent directives and Root Tag Attribute functionality#4127

Merged
SteffenDE merged 6 commits intomainfrom
sd-macro-component-directives
Jan 26, 2026
Merged

Add MacroComponent directives and Root Tag Attribute functionality#4127
SteffenDE merged 6 commits intomainfrom
sd-macro-component-directives

Conversation

@SteffenDE
Copy link
Copy Markdown
Collaborator

Supersedes #4116.

@SteffenDE SteffenDE force-pushed the sd-macro-component-directives branch 3 times, most recently from 4d8320d to 2524b4c Compare January 23, 2026 15:48
@green-david
Copy link
Copy Markdown
Contributor

@SteffenDE You are awesome!! Thank you for all your hard work on this feature this week! I will take a pass through this PR this evening and work on porting the ColocatedCSS work based on this functionality this weekend :)

@SteffenDE SteffenDE force-pushed the sd-macro-component-directives branch from 2524b4c to 5509234 Compare January 23, 2026 17:00
Base automatically changed from sd-tagcompiler to main January 23, 2026 17:01
Copy link
Copy Markdown
Contributor

@green-david green-david left a comment

Choose a reason for hiding this comment

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

This looks great overall - I left some comments, primarily things referencing the old root_tag_annotation initial pass at things 👍


Expected global :root_tag_attribute to be a string, got: #{inspect(configured_attribute)}

The global :root_tag_attribute is typically used for `Phoenix.LiveView.ColocatedCSS` and
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if for this PR we should leave out mention of ColocatedCSS and just have this say The global :root_tag_attribute is usually configured to "phx-r" and then in the PR adding ColocatedCSS I can update the message to include stuff about ColocatedCSS?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think this is fine, since we're planning to add it as the next step anyway (and we don't plan to do a release with those changes, so there's enough time to iron it out if needed later on)

@SteffenDE SteffenDE force-pushed the sd-macro-component-directives branch from 5509234 to 05a466f Compare January 24, 2026 16:14
SteffenDE and others added 5 commits January 24, 2026 17:20
Supersedes #4116.

Co-Authored-By: David Green <134172184+green-david@users.noreply.github.com>
Co-Authored-By: David Green <134172184+green-david@users.noreply.github.com>
Co-authored-by: David Green <134172184+green-david@users.noreply.github.com>
Co-authored-by: David Green <134172184+green-david@users.noreply.github.com>
Co-Authored-By: David Green <134172184+green-david@users.noreply.github.com>
@SteffenDE SteffenDE force-pushed the sd-macro-component-directives branch from 7b98f44 to 2833181 Compare January 24, 2026 16:20
@SteffenDE SteffenDE marked this pull request as ready for review January 24, 2026 16:21
@SteffenDE
Copy link
Copy Markdown
Collaborator Author

@green-david thank you for reviewing, I addressed most of the comments! I forgot to rebase yesterday after merging, so the PR contained all of the commits from #4125 as well - sorry about that.

@SteffenDE SteffenDE requested a review from josevalim January 24, 2026 16:23
Copy link
Copy Markdown
Contributor

@green-david green-david left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Copy Markdown
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

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

I added some comments but they are not blockers, as they can be moved around after the fact.

@SteffenDE SteffenDE merged commit cbfd39a into main Jan 26, 2026
16 checks passed
@SteffenDE SteffenDE deleted the sd-macro-component-directives branch January 26, 2026 11:36
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