Skin tooltip

[itsdouges]

There isn't a tooltip for skins atm

[esunder]

Hi Madou, I found your repo looking to help contribute towards hacktoberfest. Could you clarify this issue a little bit more? Where do I go to see a skin missing its tooltip? Are you looking for it behave like when you mouse over skills/gear like on the embeds example page

[itsdouges]

Hey mate!

Almost! This issue is just surrounding the skins tooltip, the skins embed is another issue (and tbh there isn't any community need for skins embed).

Currently there is no skins tooltip, so when you hover over a skin, nothing happens! Examples of skins currently in the armory would be skins you have to collect to unlock achievements.

I'm not currently on a PC (away from home until tomorrow) so it's a little hard for me to get an example achievement for you.

If you can wait 24 hours I can get you a detailed explanation on what

[itsdouges]

Needs to happen.. :)

[esunder]

Yep, I can wait. Thanks!

[itsdouges]

hi @esunder

For an example, check out the Basic Collections achievements. Tip of the Spear:

Each of those items are skins, but currently have no tooltip! They are the Gw2Skin component, which in turn is just a small wrapper over the Item component. Unfortunately the Item component is getting pretty beefy, it's really due for a refactor to just take in one set of data, but probably not right now for this issue. Note the tooltipType prop, you'll want to set this to skins for the new skin tooltip you'll make.

To get started (in no particular order):

1. You'll want to create a new skins tooltip component, look at the Items Tooltip for inspiration. They are a very simple tooltip: Just with some style differences. I'm happy for it to be its own standalone component.
2. After creating it you'll want to add it to the switch statement that decides what tooltip to show, look at the Tooltip component.
3. Modify the Gw2Skin and Item components to support skin tooltip data. Easiest thing I can think of is inside the Item component where it builds the tooltip data, add a if tooltipType === skins, set the data accordingly.

Good luck! Let me know if anything else needs explaining. Tests are also setup with mocha/enzyme, so if you're feeling frisky feel free to do some tdd too.

[esunder]

@Madou Do I need to do anything special to get achievements to load locally? I see that the dailyGroups comes back undefined in fetchAchievementGroups. My local config is pointed at the live backend. Do I need to run the backend locally instead?

[esunder]

I did some more investigation. It's not supposed to go against the gw2armory, but the guildwars2 official API, so the config shouldnt be an issue.

I put a log in the readAchievementGroups call. It looks like this is the function that is responsible for making the actual request to get the data, however my log is never triggered.

I am now trying to figure out how this call is wired up to the fetchAchievementGroups function I previously pasted.

If I am understanding the code above correctly, it says that the call can accept either an Array OR the string 'all'. However, in the fetchAchievementGroups it is calling with ['all'], which is an Array.

I thought this might be why call is not wiring up properly, perhaps there is some kind of binding of fetchAchievementGroups to the readAchievementGroups function, and because its being called with an Array instead of an Array OR string, then it fails to call the appropriate function. However, changing it to just pass the string 'all' still fails to trigger my log.

[esunder]

Ok, I think I have a fix for this: #349

But I get the following errors when trying to commit. This is on a fresh feature branch off of master in my fork. I understand and like the idea of pre-commit hooks to validate the commit, but does this happen on your machine too? Why is master in this 'broken' state? Should I be working off of dev/react16 instead?

[itsdouges]

@esunder did you install deps with yarn or npm? if you used npm it would explain it, they probably installed a newer dependency for the eslint config, thus getting new errors.

nice catch with the achievement stuff, i'll have a look at your PR.

(just took a look at the readme, whoops! I never updated it to mention yarn, sorry! fixed it now.)

as a side, I've actually started extracting out common functionality between the armory website + armory embeds, see: https://github.com/madou/armory-component-ui/

if you'd like you can add the skin tooltip there (will be easier to develop with, too).

I can worry about back porting it back to the armory :).

[esunder]

@Madou Here is a work in progress!

Here is a screenshot of the in-game tooltip. I have some questions below.

If so, couple questions:

1. Is your vision for armory to match the in-game tooltip?
2. Would you like me to make the text white? It doesnt look like it would match the rest of the armory style.
3. How might I be able to get hint text? Is that available from an API anywhere that you know of? I looked at the achievement data and didnt see hint text in there.
4. Do you have support for determining character usability?

Otherwise, if you like what I've got, I can package it up and create a PR.

[itsdouges]

@esunder nice work so far!

1. yes, but not 100%. taking some leeway to make things look better is preferred (and to keep with what we currently have)
2. yes (i think thats ok)
3. not sure! doesnt look like the api supplies it (it would make sense if the achievement supplied hint text but it doesnt look like it, ill make a issue on arenanet api cdi
4. not really. (it could just be a hard coded map anyway, not too hard) - however it doesnt really make sense for us to implement it anyway, since we don't really browse in the context of a character. so its ok to not worry about it.

i'll do a code review when you're ready :)

[itsdouges]

@esunder howd you go?

[esunder]

Here is the white "skin type" text. I am on the fence about changing the color of the skin name. Do you know if there are skins with different rarities?

I will not add character usability for now.

I am creating the PR.

[itsdouges]

@esunder i believe skins dont fall into that category (only items). so i reckon make the title white as well :)

[esunder]

Ok, will do that right now.