This PR refactors the marketing site's mini-tools to follow Hotwire and Rails conventions.
Previously, the tools used a bespoke template rendering system instead of Hotwire (Turbo)'s built-in frames. Leveraging Hotwire makes the code:
Cleaner, because we don't need to manually parse form data, or manually render a template, or define that template. Instead, we can just submit the form and let Turbo do the heavy lifting.
OSS-friendly, because it now looks like any other Hotwire codebase. Contributors will recognize patterns from the Hotwire docs and other Hotwire codebases.
More Rails-like, since we're now rendering plain old erb (e.g. <%= tool.inflation_rate %> instead of <span t-text="inflationRate"></span>)
Also, all calculations were previously done on the client. They're now happening on the back-end. This makes the code:
Simpler, because there are less moving parts since we don't need to pass data to the client for processing.
Easier to read, because of Ruby's expressiveness and pithy syntax.
Easier to maintain, as I've taken the opportunity to remove some indirection.
Testable (although this PR doesn't have any tests (yet?)) — testing server code with unit tests is easier than emulating a headless client with system tests.
Some JS cleanup was done in passing. But I decided not to make it a priority because the scope is already large enough and the existing code is not significantly breaking convention. There's room for improvement, but it's less crucial than the other problems addressed by this PR. That means there will still be work to be done on JS after merging. Most notably, the time series charts have not yet been refactored to be reusable — they remain coupled to specific tools.
Finally, I've removed code comments — which seem to have been added recently, and intentionally. I'm happy to bring those back, but I want to make the case that code comments add a maintainability burden in keeping them up to date. Ideally, the code should be readable enough that comments are mostly not necessary.
[!IMPORTANT]
This touches every single mini-tool. We should do some manual QA on every tool to ensure consistency.
Steps to add a new tool (for future reference)
Add tool to db/seeds/seeds.rb
Run bin/rails db:seed
Create a presenter under app/presenters/tool.
Class name must match the tool's slug.
All tool presenters must implement the #active_record method to find their corresponding tool in the DB.
See neighboring presenters for reference.
Create a partial under app/views/tools/widgets
Partial name must match the tool's slug.
Currently, all tools are further broken down in app/views/tools/widgets/forms and app/views/tools/widgets/content. It's a nice pattern, but not mandatory.
See neighboring partials for reference.
Allow-list the presenter's params in ToolsController#tool_params.
You're set! Use ruby to implement business logic in the presenter. Render as usual:
zachgoll
commented
1 week ago
WIP — 95% done
You'll notice some commented code, which I plan to refactor away. There are three tools to go:
Home affordabilityEarly Mortgage Payoffcloses https://github.com/maybe-finance/marketing/issues/133
This PR refactors the marketing site's mini-tools to follow Hotwire and Rails conventions.
Previously, the tools used a bespoke template rendering system instead of Hotwire (Turbo)'s built-in frames. Leveraging Hotwire makes the code:
<%= tool.inflation_rate %>instead of<span t-text="inflationRate"></span>)Also, all calculations were previously done on the client. They're now happening on the back-end. This makes the code:
Some JS cleanup was done in passing. But I decided not to make it a priority because the scope is already large enough and the existing code is not significantly breaking convention. There's room for improvement, but it's less crucial than the other problems addressed by this PR. That means there will still be work to be done on JS after merging. Most notably, the time series charts have not yet been refactored to be reusable — they remain coupled to specific tools.
Finally, I've removed code comments — which seem to have been added recently, and intentionally. I'm happy to bring those back, but I want to make the case that code comments add a maintainability burden in keeping them up to date. Ideally, the code should be readable enough that comments are mostly not necessary.
Steps to add a new tool (for future reference)
db/seeds/seeds.rbbin/rails db:seedapp/presenters/tool.#active_recordmethod to find their corresponding tool in the DB.app/views/tools/widgetsapp/views/tools/widgets/formsandapp/views/tools/widgets/content. It's a nice pattern, but not mandatory.ToolsController#tool_params.<%= tool.some_financial_indicator %><%= number_with_delimiter(tool.some_financial_indicator) %><%= number_to_currency(tool.some_financial_indicator) %>