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

Hide all fields of widget_core!() #470

Merged
merged 8 commits into from
Jan 23, 2025
Merged

Hide all fields of widget_core!() #470

merged 8 commits into from
Jan 23, 2025

Conversation

dhardy
Copy link
Collaborator

@dhardy dhardy commented Jan 23, 2025

Motivation

Previously, fields id: Id and rect: Rect were public; now they are hidden and renamed. The CodeData type is also hidden and renamed. This leaks fewer implementation details into the public API.

Details

Accessor methods already existed for both while id is only assigned to from macro-generated code, so the only missing part was the ability to assign to rect, now achieved via a new widget_set_rect! macro.

Additionally, fn TileExt::id is moved to fn Tile, with some doc changes to related methods and types.

Further goals

I would prefer to remove the rect field entirely, as follows:

  • Move fn rect() to trait Layout, implementing for all visitors (Single, Align, Pack and Margins can simply forward to their inner layout while Frame and Button already store the input rect, thus only List, Float and Grid need additional storage).
  • Implement fn rect via the layout visitor for all make_layout widgets and all user-defined widgets using the layout property.
  • Require the user implement fn rect() in other cases. (Or possibly provide this where the widget_set_rect! macro is called, which it is possible to detect before generating the widget_core!() type.)

There is, however, a blocker: fn rect() must take &self while the layout visitor currently requires &mut self (see #466). Unless/until that is resolved, this approach appears preferable.

@dhardy dhardy merged commit e929dba into master Jan 23, 2025
5 checks passed
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.

1 participant