feat: support full attribute syntax on components#220
Open
dvaergiller wants to merge 1 commit into
Open
Conversation
4de0ab0 to
2ba024c
Compare
Bring component invocation syntax to parity with regular elements:
- shorthand id (#id) and class (.class) including hyphenated values
- toggled attributes/classes/ids via [cond]
- boolean toggle attributes
- ident-valued attributes
ComponentDiv {...} now renders identically to div {...}
2ba024c to
eec4b88
Compare
Owner
|
Thank you, this looks great! I will give this a review over the weekend. :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hello!
I have a proposal for how the semantics for component attributes could be made
to enable the same syntax. The change you made earlier introducing bon to build
the components made this a whole lot easier because it allowed us to use the
.maybe_<attr>()builder function regardless if the parameter is an Option ornot. That was a blessing.
This PR contains an implementation of it and some tests to make sure
non-component elements and components can be called with the same attribute
syntax and render identically to each other.
There are cases (dynamic class lists and mixed group expressions) where a tradeoff
had to be made. In order to keep the call syntax transparent to the component itself
there are a couple of cases where a string needs to be allocated and the attribute
rendered to it. I cover that a bit more below.
Please let me know what you think!
Regular toggled attributes
An attribute value can be toggled like
Component attr=<value>[bool_expr]{}.That will expand to:
Or with an
Option<T>usingComponent attr=[<optional_value>]:Boolean attributes
A boolean attribute can be either defined as
boolorOption<bool>in thecomponent struct. When invoking a component with an empty attribute with no
toggle it will expand to:
Invoking it with a toggle will expand to:
Id attribute
If the component has an
idattribute, maud syntax now allows shorthand syntaxlike
Component #some-id, orComponent #some-id[<bool_expr>].Classlist
This one was a bit of a dilemma since the classlist is a
Vec<Class>where someof the elements might be literals and some might be dynamic expressions. It
would be possible for the component to just take an
impl Renderablebut thatdid not seem very ergonomic, and more importantly, will not compile if there is
not class set because the compiler won't be able to resolve the type.
Also, if all classes in the class list are static literals it would be nice to
pass them as
&'a str. So I opted for the classlist to be&'a strorOption<'a str>in the component. But if there are any dynamic elements in theclass list then the only way I could find for that to work while still allowing
&'a strin the component was to render the class list into an owned string andpassing a reference to it to the component.
That means memory allocation in the case where classlist is used and not all
elements are static.
Component .first-class .second-class .third-class {}expands to:All good, but
Component .first-class .second-class[<boolean>] .third-class {}needs to be dynamically built:
And then in the builder:
Also I changed the parser for class list because the current parser parsed each
class as a group expression which I do not think is necessary (that is, the
class list was a vector of groups).
Group expressions
This is similar to the classlist because the group may or may not contain
dynamic expressions and the choice is either to pass it as a renderable to the
component which has the type resolution problem and also makes it clunky in the
component definition.
I think it is nice that the component can take an attribute as a
&'a strandthen the caller without worrying about whether it was called with a literal, an
expression, or a group with a mix. But that means the caller would need to
render any group expression (and allocate).
Component attr={"one " (<dynamic_expr>) " three"}expands to:And then in builder:
I think that is a fair compromise to support the syntax while making it
transparent to the component whether it was invoked with one syntax or another.