Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: removed 'data-focus-*' attributes, removed rendering methods
  • Loading branch information
Denny09310 committed Sep 15, 2025
commit 161867ef8803c3ec66bd36bd7eb5ceb584441748
24 changes: 6 additions & 18 deletions src/LumexUI/Components/User/LumexUser.razor
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,12 @@
Class="@GetStyles( nameof(S.Base) )"
Style="@RootStyle"
data-slot="base"
data-focus=""
data-focus-visible=""
@attributes="AdditionalAttributes">
<LumexAvatar />
<div class="@GetStyles( nameof( S.Wrapper ) )" data-slot="wrapper">
@_renderName
@_renderDescription
</div>
</LumexComponent>

@code
{
internal void RenderName( RenderTreeBuilder __builder )
{
<span class="@GetStyles( nameof( S.Name ) )" data-slot="name">
@if( NameContent is not null )
@if ( NameContent is not null )
{
@NameContent
}
Expand All @@ -31,12 +21,9 @@
@Name
}
</span>
}

internal void RenderDescription( RenderTreeBuilder __builder )
{
<span class="@GetStyles( nameof( S.Description ))" data-slot="description">
@if( DescriptionContent is not null )
<span class="@GetStyles( nameof( S.Description ) )" data-slot="description">
@if ( DescriptionContent is not null )
{
@DescriptionContent
}
Expand All @@ -45,5 +32,6 @@
@Description
}
</span>
}
}

</div>
</LumexComponent>
17 changes: 1 addition & 16 deletions src/LumexUI/Components/User/LumexUser.razor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,8 @@ public partial class LumexUser : LumexComponentBase, ISlotComponent<UserSlots>
/// </summary>
[Parameter] public UserSlots? Classes { get; set; }
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a missing property for the 'Avatar Props', but I'm unsure how to handle the class. Should I use a dedicated class? Or there are already some examples in the codebase?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to think about that...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually sad :D I see a couple of options:

  • Create the AvatarProps class and pass it as parameter. Flaw: Avatar props duplicates + need to always keep them synced;
  • Create the AvatarProps parameter of type Dictionary<string, object> and pass it via attribute splatting (@attributes=...) to LumexAvatar. Flaw: we lose strict typing and intelliSense;
  • Create the AvatarContent parameter of type RenderFragment to allow consumers define avatars themselves.

I think that the last option is the best here.

I have plans to move to the following structure in the future:

<Alert>
  <Alert.Icon />
  <Alert.Content>
    <Alert.Title>Success</Alert.Title>
    <Alert.Description>Your changes have been saved.</Alert.Description>
  </Alert.Content>
  <Alert.Close />
</Alert>

, where each slot is a separate component that can be styled and arranged independently.
This is something shadcn has been doing for a while alredy, and HeroUI v3 moved to the same concept. With this approach we will gain maximum flexibility as well.

Copy link
Copy Markdown
Contributor

@desmondinho desmondinho Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I just realized that LumexChip already does that (AvatarContent approach) :D Can't remember own library solutions haha

Copy link
Copy Markdown
Contributor

@desmondinho desmondinho Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But don't wrap AvatarContent with the condition as I did -- it does not make sense.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have plans to move to the following structure in the future, where each slot is a separate component that can be styled and arranged independently.

Oh fantastic! I love this approach 🚀

Ok, so for now I add this 'AvatarContent' as RenderFragment

Copy link
Copy Markdown
Contributor Author

@Denny09310 Denny09310 Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it cool for you, if after this component is reviewed I make a branch to try and convert all the components with old styling method to the new? Just to have the project prepared for the architectural shift. I'd like to help, but I don't want to be too overwhelming 😁

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it cool for you, if after this component is reviewed I make a branch to try and convert all the components with old styling method to the new? Just to have the project prepared for the architectural shift. I'd like to help, but I don't want to be too overwhelming 😁

Yes, I’m totally fine with it! I really appreciate your input. I will do my best to make the process as smooth as possible for you.

I have to let you know that the utility for the new styling method is far from ideal, so we’ll most likely need to update it a bit.


private readonly RenderFragment _renderName;
private readonly RenderFragment _renderDescription;

private Dictionary<string, ComponentSlot> _slots = [];

/// <summary>
/// Initializes a new instance of the <see cref="LumexUser"/>.
/// </summary>
public LumexUser()
{
_renderName = RenderName;
_renderDescription = RenderDescription;
}

/// <inheritdoc/>
protected override void OnParametersSet()
{
Expand All @@ -74,10 +62,7 @@ protected override void OnParametersSet()
}

var user = Styles.User.Style( TwMerge );
_slots = user( new()
{

} );
_slots = user();
}

[ExcludeFromCodeCoverage]
Expand Down
1 change: 1 addition & 0 deletions src/LumexUI/Styles/User.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public static ComponentVariant Style( TwMerge twMerge )
.Add( "rounded-small" )
.Add( "outline-solid" )
.Add( "outline-transparent" )
// focus ring
.Add( Utils.FocusVisible ),

[nameof(UserSlots.Wrapper)] = "inline-flex flex-col items-start",
Expand Down