-
Notifications
You must be signed in to change notification settings - Fork 48
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
MaxTools #1667
base: master
Are you sure you want to change the base?
MaxTools #1667
Conversation
I don't really like that this module is using an external API. This is not a ML model and can be calculated internally |
Hi @minj. |
Hi @minj. Good afternoon. Did you see MaxDareDevil's justification for using an API? You could see the plugin code? |
Hi, I just renamed the account name here to be the same used at HT. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the whole, OK, minor improvements necessary.
Let me know, if you will be addressing my comments yourself.
Alternatively, GitHub now has a feature to allow access for the repo owner (i.e. me) to push into a PR branch on the fork (i.e. your repo)
content/foxtrick.properties
Outdated
|
||
# MaxTools | ||
module.Maxtools.desc=Show last match U20/U21/Senior | ||
module.Maxtools.RomanNumberEdition.desc=Use Roman numerals in the championship edition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "for the championship"
content/foxtrick.properties
Outdated
module.Maxtools.desc=Show last match U20/U21/Senior | ||
module.Maxtools.RomanNumberEdition.desc=Use Roman numerals in the championship edition | ||
module.Maxtools.YouthPlayers.desc=Show last match U20/U21/Senior in the Youth Player Details page. | ||
module.Maxtools.SeniorPlayers.desc=Show last match U20/U21/Senior in the Senior Player Details page. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "on the <...> page"
content/foxtrick.properties
Outdated
# %3 = phase, e.g. CC | ||
# %4 = WC number, e.g. XIV | ||
# %5 = MaxTools.match | ||
MaxTools.templateShortWithoutTable=%1: %2 - %3 %4 - %5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: EOL missing
//console.log('chegou aqui'); | ||
let lang = Foxtrick.Prefs.getString('htLanguage'); | ||
//let prefix = 'http://www.fantamondi.it/HTMS/index.php' + | ||
// `?page=htmspoints&lang=${lang}&action=calc`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
superfluous comments
let lang = Foxtrick.Prefs.getString('htLanguage'); | ||
//let prefix = 'http://www.fantamondi.it/HTMS/index.php' + | ||
// `?page=htmspoints&lang=${lang}&action=calc`; | ||
let prefix = 'https://ht-mt.org/nt/perfect-age'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use HTMT_PATH
else if (isPlayersPage || isYouthPlayersPage) | ||
module.runPlayerList(doc); | ||
else if (isTransferResultsPage) | ||
module.runTransferList(doc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
method missing
container.appendChild(getLink(player.age.years, player.age.days)); | ||
container.appendChild(doc.createTextNode(' ')); | ||
|
||
champsList.forEach(function (championship) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this block also repeats itself => extract and reuse
@@ -225,7 +224,6 @@ | |||
<script type="application/x-javascript" src="./shortcuts-and-tweaks/transfer-search-filters.js"></script> | |||
<script type="application/x-javascript" src="./shortcuts-and-tweaks/transfer-search-result-filters.js"></script> | |||
<!-- end categorized modules --> | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file (and others) should not be edited manually, we have module-update.py
for it
@@ -292,6 +292,12 @@ pref("extensions.foxtrick.prefs.module.MatchReportFormat.ShowEventIcons.enabled" | |||
pref("extensions.foxtrick.prefs.module.LiveMatchReportFormat.enabled", true); | |||
pref("extensions.foxtrick.prefs.module.MatchWeather.enabled", true); | |||
pref("extensions.foxtrick.prefs.module.MatchWeather.Irl.enabled", true); | |||
pref("extensions.foxtrick.prefs.module.Maxtools.enabled", true); | |||
pref("extensions.foxtrick.prefs.module.Maxtools.RomanNumberEdition.enabled", true); | |||
pref("extensions.foxtrick.prefs.module.Maxtools.YouthPlayers.enabled", true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
valid-ids should also be updated, for now
@@ -287,7 +288,8 @@ | |||
"*://chpp.hattrick.org/*", | |||
"*://*.foxtrick.org/*", | |||
"http://www.fantamondi.it/*", | |||
"http://pastebin.com/api/*" | |||
"http://pastebin.com/api/*", | |||
"https://ht-mt.org/*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have you considered using CORS headers instead?
Apologies for the delay. I was not in a good place mentally for weeks. |
|
@kauefelipe I am not exactly sure, what you've meant here. Btw, I'll ask for a CHPP feature to confirm this integration |
Are you still interested in working on this? I would specifically like to see these:
|
Hello minj. I was on vacation last month and I couldn't get anything because I spent a lot of time traveling. I will try to make adjustments in the next few days. |
@minj , I made some of the adjustments you asked for. I'm new to Git and I'm still learning. I gave you permission in my repository if you want to make any adjustments directly. |
Alright I hope to look at it this week but I can't make promises |
Module to show last match U20/U21/Senior