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 methods for HTML/CSS so it's easier to subclass. #693

Closed
wants to merge 1 commit into from

Conversation

davorg
Copy link

@davorg davorg commented Jun 6, 2023

I wanted to change the HTML/CSS in a subclass of this class, but using lexical variables makes it too hard. I worked around it, but a change like this would make it easier in the future.

@miyagawa
Copy link
Member

miyagawa commented Jun 6, 2023

Do you actually need to subclass for other reasons? Because we can change it to a sub callback then you could supply your custom attributes without creating a subclass.

@davorg
Copy link
Author

davorg commented Jun 6, 2023

Well, I'm currently using doing stuff with a subclass (https://github.com/davorg/plack-app-directoryindex) which adds support for index.html to Plack::App::Directory. But I wouldn't need to use that subclass if you merged #654 :-)

@davorg
Copy link
Author

davorg commented Jun 6, 2023

Oh wait. Looks like my previous PR has got somewhat corrupted. Let me clean it up and resubmit it.

@miyagawa
Copy link
Member

miyagawa commented Jun 6, 2023

Well yeah that's orthogonal :) For Plack app/middleware I generally prefer setting these things via callback attributes rather than requiring to subclass it.

@davorg
Copy link
Author

davorg commented Jun 6, 2023

Ok. Do you have a good example of how you've used callback attributes elsewhere? I'll crib off that and submit a new PR.

@davorg
Copy link
Author

davorg commented Jun 6, 2023

Replaced by #695

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