Skip to content

Conversation

@appleneko2001
Copy link
Collaborator

@appleneko2001 appleneko2001 commented May 1, 2025

Details

Add Hyperlink themes, which it doesn't existed before.

Static Resource key: MaterialHyperlinkButton
Commit: d81e934

Dark theme
image

Light theme
image

Adds two new brushes:

  • MaterialHyperlinkBrush brush for initial state hyperlinks
  • MaterialHyperlinkVisitedBrush brush for visited (clicked) state hyperlinks

Adds two new colours:

  • MaterialHyperlinkColor initial hyperlink colour
  • MaterialHyperlinkVisitedColor visited hyperlink colour

Commit: 71fd15e

Tips: those colours have different between light and dark theme

Also, this PR will update Demo buttons page

@appleneko2001 appleneko2001 requested review from JanTamis and SKProCH and removed request for SKProCH May 1, 2025 13:51
@appleneko2001 appleneko2001 changed the title Add HyperlinkButton theming support Add HyperlinkButton theming support, Fix Material.Dialog May 1, 2025
@appleneko2001 appleneko2001 requested review from SKProCH and removed request for JanTamis May 1, 2025 15:04
@appleneko2001
Copy link
Collaborator Author

TODO: after PR should update wiki page about brushes with "when added"

@SKProCH
Copy link
Collaborator

SKProCH commented May 1, 2025

@appleneko2001 welcome back :)

Can you move dialog fixes to separate PR, please?

@appleneko2001
Copy link
Collaborator Author

@appleneko2001 welcome back :)

ya its been a long time i know sorry for such wait

Can you move dialog fixes to separate PR, please?

Based on which branch? i have been a while not following this repository quite losing the way lol

@SKProCH
Copy link
Collaborator

SKProCH commented May 1, 2025

Based on master. So we should have 2 separate PRs: first with HyperlinkButton and second with dialog fixes

@appleneko2001
Copy link
Collaborator Author

Based on master. So we should have 2 separate PRs: first with HyperlinkButton and second with dialog fixes

Got it im doing it

@appleneko2001 appleneko2001 changed the base branch from master to hyperlink-and-fixes-450 May 1, 2025 15:30
@appleneko2001
Copy link
Collaborator Author

could you please review this pr at least? there is not that much changes tbh @SKProCH
image

// Used for memorise which hyperlink button clicked (or just say, visited)
#region AttachedProperty : IsClicked

public static readonly AvaloniaProperty<bool?> IsClickedProperty =
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the point of this attached property, if we have an IsVisited inside a HyperlinkButton itself?

Copy link
Collaborator

@SKProCH SKProCH left a comment

Choose a reason for hiding this comment

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

Hyperlink additions looks good (except for attached property)

@appleneko2001
Copy link
Collaborator Author

appleneko2001 commented May 1, 2025

Hyperlink additions looks good (except for attached property)

Yea i know but its used for memorise visited status, which auto-saves visited state. It can be used not only hyperlink, but also any control that derives on Button

@SKProCH
Copy link
Collaborator

SKProCH commented May 1, 2025

Hyperlink additions looks good (except for attached property)

Yea i know but its used for memorise visited status, which auto-saves visited state. It can be used not only hyperlink, but also any control that derives on Button

I'm not really sure that anyone will need it in this version (especially since it is not used anywhere, even in the style for HyperlinkButton). Avalonia has a large selection of buttons, are you sure that users will need this property (and that we will support it for all existing buttons)?

I'd rather not complicate things and not create unnecessary properties that will be needed by a very limited number of people, but maybe you have a different opinion?

@appleneko2001
Copy link
Collaborator Author

appleneko2001 commented May 1, 2025

I'm not really sure that anyone will need it in this version (especially since it is not used anywhere, even in the style for HyperlinkButton). Avalonia has a large selection of buttons, are you sure that users will need this property (and that we will support it for all existing buttons)?

Well if you dont like it i can delete that and try to auto-binding IsVisited while library loading, also this ButtonAssist.IsClicked doesn't make too much problems since its attached property not direct property or what (unless IsVisitedProperty can be attached with some tricky way that i dont know)

Technically if you give it null it will suddenly returns without further operation and no saving state, if you give it false it will automatically turn IsClicked to true state

I'd rather not complicate things and not create unnecessary properties that will be needed by a very limited number of people, but maybe you have a different opinion?

Do we have other active member for this conversation?

@SKProCH
Copy link
Collaborator

SKProCH commented May 1, 2025

It's not really about "like it or not", I'm thinking more about the possible scope of application. Are you planning to make some modifications to existing button styles to take ButtonAssist.IsClicked into account? It's just that it's not used anywhere in the current implementation, and I'm not sure that it's worth including in the current PR. Maybe it's worth putting it in a separate PR, which will also show some use of this property?

I'm just saying that this increases our public API, and I don't really want to just add properties there that have no use - it will be difficult to remove them (without fear of breaking user code).

I don't see any extensive use of this property, since HyperlinkButton already has everything it needs, and for other buttons it is most often not needed, besides, it's not very clear how it should look (since there is no such thing in any material guidelines). What I mean is that we either need to come up with some specific useful application for it and display it in styles (like other properties that we create, for example TextBox.Hints), or not add it, because those few people who will need it are not the majority, we are unlikely to be able to guess their use case, and they can add it themselves.

Do we have other active member for this conversation?

Probably not, but I have never seen a request for such functionality anywhere

@appleneko2001
Copy link
Collaborator Author

How do you think then, if I put this thing to the wiki page as a extra feature, instead of including this attached property to the library ?
if I have to go such approach then how the wiki page will be looks like?

I just think this thing would help to less trouble while implementing, like its already implemented and you dont have to worry about it.

Also, I would want to expose Material.Ripple RaiseRipple method which you can easily to trigger a ripple effect by keyboard or whatever, and you can pass two parameters (2D normalised position) to customise where should be raised the effect. Its already exist on my local copy.

For example, you have used Button.ClickEvent.Raised.Subscribe() to attach a event handler that receives button clicked events, and it knows its a Button object, and then you can use such method like this:

internal static void RaiseRipplePrivate(Control c)
{
    // Try to find first RippleEffect control with name PART_Ripple
    var visual = c
        .GetVisualDescendants()
        .FirstOrDefault(a => a is RippleEffect && a.Name == "PART_Ripple");
    
    // if such control not exist or mouse is over on it 
    if(visual is not RippleEffect effect || effect.IsPointerOver)
        return;
    
    effect.RaiseRipple();
}
2025-05-02.03-20-42.mp4

@SKProCH
Copy link
Collaborator

SKProCH commented May 1, 2025

P.S. I want to add a little to my previous answer: I am not against adding something that has a non-zero chance of being used and is implemented. That is, if you make this property usable in styles (for example, implement some change for buttons styles when they are already pressed), then it will be generally fine.

How do you think then, if I put this thing to the wiki page as a extra feature, instead of including this attached property to the library ?
if I have to go such approach then how the wiki page will be looks like?

I don't really see how to do this, because the property itself will not be associated with existing styles, i.e. the user will probably add it along with style changes, which he will also make himself.... Therefore, it is probably not very connected with Material.Avalonia, rather it is simply "what can be done with attached property", but if you really want to put it on the wiki... Okay

Also, I would want to expose Material.Ripple RaiseRipple

Usually ripple triggers as a feedback for pointer pressing... But I don't mind about extending such things, you can do it in the new PR if you want.

@appleneko2001 appleneko2001 linked an issue May 1, 2025 that may be closed by this pull request
1 task
@SKProCH
Copy link
Collaborator

SKProCH commented May 1, 2025

@appleneko2001 i can separate PRs by myself, if you want

@appleneko2001
Copy link
Collaborator Author

P.S. I want to add a little to my previous answer: I am not against adding something that has a non-zero chance of being used and is implemented. That is, if you make this property usable in styles (for example, implement some change for buttons styles when they are already pressed), then it will be generally fine.

Nah I'm just thinking how to do it better than making more troubles, since this project isn't my personal project anymore, its for the community, I should take some others opinions too before do anything break changes. Thats why I said we need some others active members to review this question, and also I have choose submit changes via PR with reviews to make the changes transparent.

Also, I would want to expose Material.Ripple RaiseRipple

Usually ripple triggers as a feedback for pointer pressing... But I don't mind about extending such things, you can do it in the new PR if you want.

Ah thats because ripple effects are not only for feedback for pointer press, but also pretty helpful for mentioning users where to take the attention

Such thing or ideas are already existed in

But yea since our Ripple effect are working different it couldn't be done same as their demo looks like, but at least it does exist for such use

@appleneko2001
Copy link
Collaborator Author

appleneko2001 commented May 1, 2025

@appleneko2001 i can separate PRs by myself, if you want

Yea I leave this power to your hand instead @SKProCH , the IsClicked attached property changes can be decided before merging all branches

@SKProCH SKProCH removed a link to an issue May 1, 2025
1 task
@SKProCH SKProCH changed the title Add HyperlinkButton theming support, Fix Material.Dialog Add HyperlinkButton theming support May 1, 2025
@SKProCH SKProCH merged commit 26a8be0 into AvaloniaCommunity:hyperlink-and-fixes-450 May 1, 2025
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.

Null & InvalidCastException binding errors when opening dialogs in demo

2 participants