itsdouges/armory-app

Allow items embed to select skins

itsdouges opened this issue · 20 comments

itsdouges commented
Allow items embed to select skins
stygiansabyss commented

Any word on if this will become a thing. I had figured that something like data-armory-<id>-skin=<skin id> would have worked.

itsdouges commented

hey @stygiansabyss

it can be, im just not actively working on the armory atm (other than modifying it to be on brand for path of fire)

would you be keen to implement this?

the current problem is there isn't a tooltip for skin, thus theres no embed.

if you want a skin embed we'd need to make the tooltip for skins first, and then make the embed.

stygiansabyss commented
stygiansabyss commented
stygiansabyss commented

It looks like the component has the ability to use a skin. Currently trying to understand how this gets called. I am assuming somewhere looks for data-armory-embed and uses that to determine which modules to load. At which point it loads the creator file and passes that data to the component file?

itsdouges commented

The embed?

From start to finish (links go to lines of code):

  1. User adds the gw2aEmbeds.js script to their page
  2. DOMContentLoaded event is triggered
  3. This line of code is eventually fired, which collects all elements on the page that have a data-armory-embed attribute. The value of this attribute is the embed name.
  4. The embed is imported asynchronously here and then is instantiated and passed props
  5. Let's say we're loading the items embed. This creator is then called, does any calculations it needs to get extra data from the div, and then returns the <Items /> embed component.
  6. That being this component

For tooltip, there are two main things:

  1. The TooltipTrigger
  2. And the Tooltip itself

You won't have to worry about modifying the TooltipTrigger for this feature, but you will have to add a new case in the Tooltip switch to handle skins, as well as creating a new component for the skin content. Look at the Item tooltip for inspiration!

Hope this helps

stygiansabyss commented
stygiansabyss commented

Thank you for that. It looks like I will need to add something like character actions has inside fetchCharacter that grabs the skins and then the character component seems to add it to the skins array which is then passed to the tool tip. Just need to figure out how to get the skin from the item component.

itsdouges commented

Is this just for a skins embed?

If it is, don't worry about fetchCharacter.

What you want to do is create a skins creator, use this items creator for inspiration: https://github.com/madou/armory-react/blob/master/src/embeds/creators/items.js

Then create a Skins embed component, use Amulets https://github.com/madou/armory-react/blob/master/src/embeds/components/Amulets/index.js for inspiration.

There is already a Skin component ready for you to use: https://github.com/madou/armory-react/blob/master/src/common/components/Gw2Skin/index.js.
Make sure it sets the correct tooltipType as skin (see: https://github.com/madou/armory-react/blob/master/src/common/components/Item/index.js#L88)

Just use that component, and pass it the ids you want. The data will be fetched in the background. What's missing is the Skin tooltip! So make that!

stygiansabyss commented

Ok, so my goal was to supplement the items embed to allow for skins. So we would do data-armory-{ITEM_ID}-skin="{SKIN_ID}. Then it would use the icon of the skin with the details of the item itself. Similar to how the full character one does it.

itsdouges commented

oh! i misunderstood then, sorry :)

🤔 you'd have to bring in the action for fetching skins, pass them the ids from the embed, modify the selector in the items embed to bring the skins, and then pass the skin data down into the Items component

stygiansabyss commented

I dont see a fetching Skins action. It looks like character/actions.js is just calling a magic method of some kind on gw2/actions.js.

(sorry for the delay, had work stuff to get pushed crazy fast)

itsdouges commented
stygiansabyss commented

I know this isnt correct, but heres where I am so far.

stygiansabyss@9506b7e

error in browser is Warning: NaNis an invalid value for thewidthcss style property. Check the render method ofIcon. and the skin is still not being used. But if I log the skin in item index it does get a valid skin object.

itsdouges commented

ill have a look when i get a chance

itsdouges commented

dw about the error in the browser, thats an existing thing

stygiansabyss commented

I added a comment on the review. If you get a chance, could you look at it? Thanks sir

itsdouges commented

@stygiansabyss hey mate, sorry I haven't been that helpful.

i ended up implementing this for you, check out the pull request: #344

just need to fix the build and add some unit tests.

check it out ! :)

itsdouges commented

merged.