search

gergelyszabo94 / csgo-trader-extension

CSGO Trader Browser Extension to help with CS:GO item trading, marketing and much more
https://csgotrader.app
GNU General Public License v3.0
213 stars 42 forks source link

Discord Embeds #387

Closed hexiro closed 3 years ago

hexiro commented 3 years ago

I think it turned out pretty good. Please let me know if you find any bugs. I tried by best to bug test, but I found it difficult in the setting of an extension. Also, one of my visual studio code extensions changed the ordering of some of your import statements, so you might want to run the linter.

In regards to the embed, Maybe the CSGO Trader and icon can be replaced with the steam user's name and icon, as the CSGOTRADER.APP username is already there, and I didn't take that into account when I initially created the embed.

Let me know what you think!

image image

hexiro commented 3 years ago

another example i just got image

gergelyszabo94 commented 3 years ago

Thanks, I am testing it. The first thing that striked me is that I think the link should stay at the bottom. The first lines of the message or important realestate, that is what gets previewed when getting notifications on mobile/smart devices.

hexiro commented 3 years ago

Sure. I'll try and find something better.

hexiro commented 3 years ago

If I move it to the bottom it's not clickable, so maybe something like the 2nd image will work?

image image

hexiro commented 3 years ago

You can also link to sender's steam profile, but then the "Offer" text looks bare on it's own. image

gergelyszabo94 commented 3 years ago

Making the link just a text makes a lot of sense, it looks weird on its own though. You can only embed one image and the position if fixed, right?

hexiro commented 3 years ago

Unfortunately, the author (top here) can have a url set, but the footer on the bottom can't as far as I know. Also, it looks like only the description (which has the Offer ($0) here) and title support markdown.

hexiro commented 3 years ago

If it's possible to get what time the trade was sent with that function, a timestamp could be set in the embed. image

gergelyszabo94 commented 3 years ago

If it's possible to get what time the trade was sent with that function, a timestamp could be set in the embed. image

This is what a tradeoffer object looks like image I think we need "time_updated" (I think it should be when it was confirmed for incoming offers). It should be avaiable inside the "notifyAboutOfferOnDiscord" function so we can use it or pass it down.

Btw I think you have introduced a typo "Recieving".

hexiro commented 3 years ago

Alright cool I'll get that sorted soon!

hexiro commented 3 years ago

This is my final design draft as of now. What do you think about this? The link I'm hovering over is the link to the tradeoffer.

image

gergelyszabo94 commented 3 years ago

It looks great, it's much cleaner without the full url showing. Altough you missed the profit/loss from the "title", not sure if it was intentional but I definitiely need it (also already thought about putting the percentage there as well) to quicly see important it is to check the trade.

If you feel like it's final then I can pull this and add those myself.

hexiro commented 3 years ago

Just so you know, technically the "title" is actually the "author" based on discord's embed names. I chose to do this because If you set a URL on the title it makes the text turn blue, and I don't think the blue matched well with the orange. I didn't intentionally remove the profit/loss so I'll add that as a final touch.

hexiro commented 3 years ago

cool! was fun working on this. Super practical extension for csgo traders! glad to be a part of it!

gergelyszabo94 commented 3 years ago

While it looks great on desktop and mobile, the author is completely missing from the mobile notification preview. We have to work around this before it goes into production. image

hexiro commented 3 years ago

What do you suggest I do? I can move the embedded link back into the description which should show up on mobile I believe.

gergelyszabo94 commented 3 years ago

The only problem with that is the blue color, right? I think we can live with that.

hexiro commented 3 years ago

Is it okay if the trade message stays in the description? That would show up before the giving / receiving.

gergelyszabo94 commented 3 years ago

I think we are going to have to put the link at the end, still not perfect on mobile. image

hexiro commented 3 years ago

From my testing, If I have the title, or the description of the embed set, the fields (being the giving & receiving) don't show in the mobile notification. This means there is a big tradeoff. To optimize for mobile you can either choose to show the title & description, or the giving / receiving. Let me know what you think.

In my opinion, I'd say the title & description are more important, as they show how much you profit / loss and the message included by the partner. You can then click on that notification to see the items in which they propose and go from there.

hexiro commented 3 years ago

This is my current design with the mobile notification. I think it looks good, but there is a lot of information with not a lot of space, so let me know if you want any changes.

August-10-2021 18-12-43 image

gergelyszabo94 commented 3 years ago

I think the sender's name would be useful and would still fit.

hexiro commented 3 years ago

In the example i left i include the partner at the top, which I don't believe can be included in the embed.

gergelyszabo94 commented 3 years ago

Ohh, okay, I did not realize that is what it was.

hexiro commented 3 years ago

gotcha gotcha. I'll commit that now

hexiro commented 3 years ago

alright :) let me know what you think.

Looskie commented 3 years ago

LETS GOOOO THANKS FOR MERGE