Replace the input! macro with an Input builder #646
No reviewers
Labels
No labels
A: API
A: Backend
A: Federation
A: Front-End
A: I18N
A: Meta
A: Security
Build
C: Bug
C: Discussion
C: Enhancement
C: Feature
Compatibility
Dependency
Design
Documentation
Good first issue
Help welcome
Mobile
Rendering
S: Blocked
S: Duplicate
S: Incomplete
S: Instance specific
S: Invalid
S: Needs Voting/Discussion
S: Ready for review
Suggestion
S: Voted on Loomio
S: Wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: Plume/Plume#646
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "input-builder"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Pros:
Cons:
Codecov Report
this looks pretty cool!
Maybe implementing https://docs.rs/ructe/0.6.2/ructe/templates/trait.ToHtml.html would make more sens than a stand-alone function? Also, clippy is complaining about conventions, to_* usually take by reference
I thought about it, but my function takes a
gettex::Catalog
, whileToHtml::to_html
doesn't. I could have passed it as an argument toInput::new
tho.But I made the commit before reading your review, so it is
Input::html
now (I can still change it if you prefer).definitely, what @fdb-hiroshima said
update / carification:
About implementing Ructe's
ToHtml
trait.I did not notice the Catalog. It was just a suggestion, but as it's totally fine as is
As
.html()
take the Catalog, and the provided label in Input::new is always translated, can't it be translated transparently from.html()
instead of explicitly each time? (I don't know if it would be better or worst, that's an actual question)gettext-macros requires strings that are known at compile time, so that it can add them to the .pot file. With the latest version of the crate (which requires Rust 2018 for some reason, so we can't use it now), there is a new macro, called
t
that would allow us to do:Which would be a bit better (but we can't really do anything else than that I think, these strings should be marked as translatable with a macro at some point).
👀
I think error is no longer put in a
<p class="error" dir="auto">{}</p>
@ -13,0 +13,4 @@
.default(&form.title)
.error(&errors)
.set_prop("minlength", 1)
.html(ctx.1))
previously had the
required
property@ -18,3 +18,3 @@
<form method="post" action="@uri!(instance::update_settings)">
@input!(ctx.1, name (text), "Name", form, errors.clone(), "props")
@(Input::new("name", i18n!(ctx.1, "Name"))
what was
props
, why is it gone?@ -13,0 +13,4 @@
.default(&form.title)
.error(&errors)
.set_prop("minlength", 1)
.html(ctx.1))
mb, it's added automagically when not optional
@ -18,3 +18,3 @@
<form method="post" action="@uri!(instance::update_settings)">
@input!(ctx.1, name (text), "Name", form, errors.clone(), "props")
@(Input::new("name", i18n!(ctx.1, "Name"))
I don't know to be honest, but it clearly was here by mistake (this argument was for additional HTML properties).
👍