Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement contextual icons with clickable actions #62

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Pluckerpluck
Copy link
Contributor

Continuing on from your existing work, this enables the contextual icons and makes them clickable. Currently the achievement icon is just informative (that the marker is tied to an achivement), while the wiki icon is clickable, and will open the browser to the wiki.

Do check this works, as I run System.Diagnostics.Process.Start(url); to open the web page, which works fine for me, but is one of those weird things that may not for you.

Tooltips are not yet enabled.


Regarding implementation, it turns out onClick doesn't fire when a field is disabled, but to me it made sense that these icons still be clickable. So at least for now I've overriden OnLeftMouseButtonReleased. It also doesn't check the exact bounds, but instead looks for the click within the 18px window that frames the icon (so no gap between the icons) and uses the full height of the menu item.

I also decided to change your empty Actions into null. This allowed for me to implement a check where a null Action indicates that the icon should have no click functionality, and thus just click through to the underlying menu item. This felt better to me than having an icon that clicking it just did... nothing.

For the URL for the search, I just use the achievement named URL encoded into the same format that GW2 Efficiency uses for their search. I assume this is good in 99% of cases. Absolutely no idea how it handles achievements that share the same name (if they exist).


Anyway, tell me if you want any other changes.

- Activated achievement icon
- Activated wiki icon, and implemented click to launch web page
@Pluckerpluck
Copy link
Contributor Author

Pluckerpluck commented Jan 3, 2022

This pull request has the side effect of enabling the BasicTooltipText

PathingModule.Instance.Gw2ApiManager.Gw2ApiClient.V2.Achievements.GetAsync(achievementId).ContinueWith((achievementTask) => {
    this.BasicTooltipText = $"[Achievement]\r\n\r\n {DrawUtil.WrapText(GameService.Content.DefaultFont14, achievementTask.Result.Description, 300)}\r\n\r\n{DrawUtil.WrapText(GameService.Content.DefaultFont14, achievementTask.Result.Requirement, 300)}";
}, TaskContinuationOptions.NotOnFaulted);

I don't know enough about this system to know when this BasicTooltip is shown vs the full tooltip. It kind of looks like it only shows in situations where the regular tooltip hasn't finished rendering? But I thought I'd point it out. You'll want to comment out those lines if you wish to disable this.

@dlamkins
Copy link
Member

dlamkins commented Jan 5, 2022

This pull request has the side effect of enabling the BasicTooltipText

PathingModule.Instance.Gw2ApiManager.Gw2ApiClient.V2.Achievements.GetAsync(achievementId).ContinueWith((achievementTask) => {
    this.BasicTooltipText = $"[Achievement]\r\n\r\n {DrawUtil.WrapText(GameService.Content.DefaultFont14, achievementTask.Result.Description, 300)}\r\n\r\n{DrawUtil.WrapText(GameService.Content.DefaultFont14, achievementTask.Result.Requirement, 300)}";
}, TaskContinuationOptions.NotOnFaulted);

I don't know enough about this system to know when this BasicTooltip is shown vs the full tooltip. It kind of looks like it only shows in situations where the regular tooltip hasn't finished rendering? But I thought I'd point it out. You'll want to comment out those lines if you wish to disable this.

Yes, we do not want to include this at all. It predates the implementation of the tooltip that we have now and was for testing the concept early on.

@Pluckerpluck
Copy link
Contributor Author

Ok. Just removed it. Do you have any icon that you'd think of using for "completed achievement"?

@dlamkins
Copy link
Member

dlamkins commented Jan 8, 2022

Likely a combination of the normal icon + a small ✔️ in the bottom right (like a small version of texture 154979) or something.

This looks pretty good. I can look at the textures and see if I can provide something that fits what I was imagining. I'll also give the build a test.

Regarding process.start for the URLs, yes, that's perfectly fine and exactly what we do elsewhere in Blish HUD to ensure the default browser is used.

@Pluckerpluck
Copy link
Contributor Author

Lol, there so many tick textures... 154979, 157012, 156950 and 156108.

Anyway I wanted to bring up two things:

The achievement icon uses 155062. There is an alternative with 155061 that might be worth looking into as it's a bit bigger and brighter. Depends how it looks among the others, but something I wanted to note down so I don't forget.

Otherwise we could either use
834012
which is the story mastery completed icon. Though it may result in a sizing mismatch. Would need to test it.

Or something like these two designs:
155061+156950
155062+156950

Both use the 156950 tick (which has a nice outline). The former is using the larger achievement icon, the latter is using the one already used in this pull request.

Thoughts? I can try both out and get screenshots maybe later today or tomorrow.


Then with regards to achievement disabling, would we simply be, by default, unchecking completed achivements when Blish launches regardless of the saved value? (Rather than disabling). This icon would then show you why a marker is disabled.

@dlamkins
Copy link
Member

The story master completed icon does look nice if it did fit nicely. The prior of the latter two icons also looks nice (a bit brighter - less dull).

I'm still not sure the best option about the disabling vs. unchecking vs. something else. Trying to think through what makes sense intuitively. Especially for a user trying to make a distinction between an actually unchecked category and one that is completed (despite the icon change 🤔).

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.

2 participants