-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add options contexts to set conventions for more involved definitions #3
Conversation
test/dummy/app/views/showcase/pages/templates/stimulus_controllers/welcome.html.erb
Outdated
Show resolved
Hide resolved
This is a really interesting idea! Is a For example, I think an |
Yeah, the idea is they're able to do something like this: Showcase.options.context :stimulus do
def value(name, ...)
option("data-#{@controller}-#{name}-value", ...)
end
end and then
Oh yeah, it'd be interesting to ship something like this out of the box too. |
* main: (21 commits) Prep v0.2.0 release Ensure test support for deeply nested previews Rename `Page` to `Preview` (#17) Route Engine root path to `showcase/engines#index` (#18) Add a description to our Plain Ruby sample Load with Zeitwerk (#14) Support partials with host application routes Bugfix: support deeply-nested partials in IntegrationTest Convert Plain Ruby sample to new partial location and have it auto-smoketested 😁 Add automatic Smokescreen test generation (#13) Indent Path Tree children Test Coverage for template- and partial-overrides Move `pages#index` template to `engine#index` Revert `<input type="checkbox" readonly>` change Build Tailwind assets Fail CI when Tailwind generates a diff Add HTML Test Coverage Execute Test Suite in GitHub Action (#7) Prefer partials to templates (#2) Prefix Tailwind classes with sc- to avoid clashing with app Tailwind (#4) ...
Looks like the build is failing on |
@kaspth what about a guarded backport that stashes the block argument? require "active_support/core_ext/object/with_options"
module WithOptionsBackport
extend ActiveSupport::Concern
included do
alias_method :__original_with_options, :with_options
def with_options(options, &block)
if block.nil?
options_merger = nil
__original_with_options(options) { |object| options_merger = object }
options_merger
else
__original_with_options(options, &block)
end
end
end
end
if Rails.version < "7.0"
Object.include WithOptionsBackport
end It's brittle, but the behavior itself isn't too complicated to cover. |
I noticed that |
@contexts = Hash.new { |h,k| h[k] = Class.new Context } | ||
|
||
def self.define(key, &block) | ||
contexts[key].class_eval(&block) # Lets users reopen an already defined context class. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, if people override an already defined method they may break Showcases from engines, which may rely on the default context method definition.
<% showcase.options.context :stimulus, controller: :welcome do |o| %> | ||
<% o.optional.targets :greeter, "If the id of the target element must be printed" %> | ||
<% o.required.values :yell, "Whether the hello is to be YELLED", default: false %> | ||
<% o.required.classes :success, "The success class to append after greeting" %> | ||
<% o.required.outlet :list, "An outlet to append each yelled greeter to" %> | ||
<% o.optional.action :greet, "An action to repeat the greeting, if need be" %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels pretty clean and intention revealing!
916b382
to
22e70a8
Compare
I've gone back to push more on the Bullet Train integration side of things bullet-train-co/bullet_train-core#46, so I'll be needing this soon. I'll try to document this soon and then go ahead with what we've got. |
No description provided.