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

Mod : Estimation du prix de craft #163

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

TheKetur55
Copy link

No description provided.

@JulienCoutault JulienCoutault added the enhancement New feature or request label Jan 4, 2022
Crédit : Tird, Reyem, Prout01, Aejii, Romarin
Crédit : Reyem, Aejii, Tird, Prout01, Romarin
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Le mod est sympa, je te suggère quand même quelques modifications.

  • Esthétique (kamas)
  • Technique pour le prochain gars (ou toi) qui devra regarder/modifier le code un jour

let leftdiv = document.createElement("div");
leftdiv.className = "estimationprix";
leftdiv.id = "eprix";
leftdiv.innerText = "Prix moyen du craft : " + this.formatNumber(totalprix) + " Kamas" + " (" + totalpods + " Pods).";
Copy link

Choose a reason for hiding this comment

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

Il serait peut-être + judicieux d'afficher l'icône Kamas, plutôt que le mot.
Tu peux faire quelque chose comme ça par exemple (à tester):

leftdiv.innerText = "Prix moyen du craft : " + this.formatNumber(totalprix) + " <img src='./assets/ui/icons/kamas.png' width='20px' height='16px' />" + " (" + totalpods + " Pods).";

Copy link
Author

@TheKetur55 TheKetur55 Jan 10, 2022

Choose a reason for hiding this comment

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

Je vien de reupload le script, il faudra juste que tu jete un coup d'oeil niveau css pour mettre les 3 div sur la meme ligne stp

Copy link

Choose a reason for hiding this comment

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

Ta solution me semble étrange, je regarde ça ce week-end ;)

Copy link

Choose a reason for hiding this comment

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

En fait il faut simplement faire un .innerHTML plutôt qu'un .innerText

src/app/mods/estimationprix/estimationprix.ts Outdated Show resolved Hide resolved
Il faudra juste que Broseidon jete un coup d'oeil niveau CSS
@TheKetur55 TheKetur55 marked this pull request as draft January 10, 2022 15:58
@TheKetur55 TheKetur55 marked this pull request as ready for review January 16, 2022 13:18
@TheKetur55
Copy link
Author

Aucun probleme contastater, pret a etre merge pour une beta

Changement du nom de classe pour avoir le mod depuis le menu des métier
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants