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

Properly handle shoes styles and keyword args in Lacci initialize() methods #473

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

noahgibbs
Copy link
Collaborator

@noahgibbs noahgibbs commented Dec 4, 2023

Description

Handle positional arguments, not just styles, in Drawable#initialize. That makes it obvious how to combine positional with keyword args. Use Shoes default styles for default argument values instead of initialize default params. Add some tests for default argument values.

There are a few places we still don't do this, mostly flow and stack. We still need to rework a few places where we do weird nonstandard things -- e.g. margin should change to Drawable, not separately Flow/Stack/Para. Para needs to stop passing all "extra" args as HTML attributes, which doesn't even work for the styles (margin, align) it's using it for. But those can be a later PR.

Checklist

  • Run tests locally

@noahgibbs noahgibbs force-pushed the lacci_constructors branch 2 times, most recently from 3f74051 to 46513b8 Compare December 4, 2023 22:20
@noahgibbs noahgibbs marked this pull request as ready for review December 5, 2023 08:04
@noahgibbs noahgibbs force-pushed the lacci_constructors branch 2 times, most recently from a38ccfc to 2f8a9b1 Compare December 5, 2023 10:51
@noahgibbs
Copy link
Collaborator Author

Nearly there. Schwad allows inferring radius from width/height -- so does old Shoes. But I'm not doing that yet. Need to get examples/oval.rb working (it does that) and a test that checks that behaviour.

@noahgibbs noahgibbs force-pushed the lacci_constructors branch 2 times, most recently from 1fd0a36 to a3ded3c Compare December 5, 2023 11:43
@noahgibbs noahgibbs force-pushed the lacci_constructors branch 2 times, most recently from db78cc2 to f0dd636 Compare December 5, 2023 12:35
@noahgibbs noahgibbs merged commit 9e97b32 into main Dec 6, 2023
1 check passed
@noahgibbs noahgibbs deleted the lacci_constructors branch December 7, 2023 15:10
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.

2 participants