Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Spacing into DynamicSpacing using proc macro #20504

Merged
merged 5 commits into from
Nov 11, 2024

Conversation

iamnbutler
Copy link
Member

@iamnbutler iamnbutler commented Nov 11, 2024

Density tracking issue: #18078

Post-Merge Note: It looks like the enum variant docs aren't showing up correctly. Will work on a fix for this.

This PR refactors our spacing system to use a more flexible and maintainable approach. We've replaced the static Spacing enum with a dynamically generated DynamicSpacing enum using a proc macro.

Enum variants now use a BaseXX format, where XX = the pixel value @ default rem size and the default UI density.

For example:

CustomSpacing::Base16 would return 16px at the default UI scale & density.

I'd love to find another name other than Base that is clear (to avoid base_10, etc confusion), let me know if you have any ideas!

Changes:

  • Introduced a new derive_dynamic_spacing proc macro to generate the DynamicSpacing enum
  • Updated all usages of Spacing to use the new DynamicSpacing
  • Removed the custom_spacing function, mapping previous usages to appropriate DynamicSpacing variants
  • Improved documentation and type safety for spacing values

New usage example:

.child(
    div()
        .flex()
        .flex_none()
        .m(DynamicSpacing::Base04.px(cx))
        .size(DynamicSpacing::Base16.rems(cx))
        .children(icon),
)

vs old usage example:

.child(
    div()
        .flex()
        .flex_none()
        .m(Spacing::Small.px(cx))
        .size(custom_spacing(px(16.)))
        .children(icon),
)

Release Notes:

  • N/A

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Nov 11, 2024
@iamnbutler iamnbutler changed the title New density scales Refactor Spacing into DynamicSpacing using proc macro Nov 11, 2024
@iamnbutler iamnbutler marked this pull request as ready for review November 11, 2024 16:05
@iamnbutler iamnbutler merged commit 94d8ead into main Nov 11, 2024
14 checks passed
@iamnbutler iamnbutler deleted the new-density-scales branch November 11, 2024 16:08
iamnbutler added a commit that referenced this pull request Nov 11, 2024
In #20504 the CustomSpacing enum variants ended up not having docs. This
PR fixes that, now docs correctly show for variants.


https://github.com/user-attachments/assets/8cc409c9-7b71-4c21-a538-3fd5dded3e00

Release Notes:

- N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant