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

Use a single Scarpe::App instance as self nearly everywhere #277

Merged
merged 1 commit into from
Jul 5, 2023

Conversation

noahgibbs
Copy link
Collaborator

@noahgibbs noahgibbs commented Jul 2, 2023

See issue #136 for initial bug report. See https://github.com/scarpe-team/scarpe/wiki/ShoesSlotsSelf.md for a description of the preferred behaviour.

This should fix #136 when complete and merged.

Description

This PR passes the Shoes::App instance as self to relevant APIs.

However, I did not do this for Shape. Shape is going to require a more extensive rework. I'll file a bug for it.

Checklist

  • Run tests locally
  • Run linter(check for linter errors)

@noahgibbs noahgibbs force-pushed the shoes_api_self branch 5 times, most recently from 19b60b6 to 2186904 Compare July 4, 2023 09:00
@noahgibbs noahgibbs marked this pull request as ready for review July 4, 2023 09:04
@noahgibbs noahgibbs force-pushed the shoes_api_self branch 3 times, most recently from 329f52c to 3578d0f Compare July 4, 2023 11:58
require "nokogiri"

class Scarpe::Widget
def download(url)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note to me: make sure we have a test for download. Is this on the right class?

Copy link
Collaborator

@Schwad Schwad left a comment

Choose a reason for hiding this comment

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

Paired on this

@noahgibbs
Copy link
Collaborator Author

Notes from review:

Add test that uses ivar properly, including set inside stack and used outside it.
Make sure event handler for button-click uses the right self.

@Schwad
Copy link
Collaborator

Schwad commented Jul 5, 2023

Will try to rewrite some examples without globals after

Make display properties properly inheritable
Move DocumentRoot special behavior into app, make it 'just a Flow'
Remove unused app-option override code in ControlInterface
@noahgibbs
Copy link
Collaborator Author

Okay. Added the new tests, will merge after CI builds.

@noahgibbs noahgibbs merged commit e84e321 into main Jul 5, 2023
2 checks passed
@noahgibbs noahgibbs deleted the shoes_api_self branch July 8, 2023 20:29
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.

Shoes DSL: self in blocks, Shoes::Widget, different APIs in different places
2 participants