Willy-JL / F95Checker

GNU General Public License v3.0
123 stars 19 forks source link

Extension settings + tag highlighter #115

Closed r37r05p3C7 closed 10 months ago

r37r05p3C7 commented 10 months ago

Forum highlighter was partially derived from the X version but adapted to utilize actual Tag instead of strings. Tags are highlighted both on the forum and in the app. Forum highlighter forced me to add a new field to the Tag enum, containing the plain text version of the tag. This became necessary as the Latest Updates page lacks any hrefs attached to the tags, only displaying their names. Fortunately, this change also has a positive side effect of enhancing the appearance of in-app tags, mirroring formatting of the forum.

Willy-JL commented 10 months ago

you are productive, damn.

i was planning on eventually doing these things with a tag rework as you can see in the progress tracker, was gonna be the next thing after gallery images which im doing now. i thought of making it figure out what tags exist directly from parsing the forum, as it would mean no issues when the forum adds or removes tags. but from what you say about latest updates having no coherent attribute in the page source then that wouldnt be possible, need a static database of tags, for which yours seems like the best solution. ill look through the code, but already pretty sure its gonna be ok.

im starting to get some serious suspicions that youre littleraisins on an alt account xD

Willy-JL commented 10 months ago

also, are you a js developer by trade? those list(map(lambda t: t.text, tags)) shenanigans really remind me of js devs, id think a python dev would go for the [t.text for t in tags]. just a random thought xD

r37r05p3C7 commented 10 months ago

after gallery images which im doing now

nice, i didn't think gallery would make it into 11.0.

i thought of making it figure out what tags exist directly from parsing the forum, as it would mean no issues when the forum adds or removes tags

yeah, i don't think adding more moving parts is worth it, considering how rarely tags are changed

im starting to get some serious suspicions that youre littleraisins on an alt account xD

that would be wild, but no. i figured i might as well contribute something back to the community after X died and learn some python in the process for the next project i have coming up.

also, are you a js developer by trade?

also no, fortunately writing python is the closest i've come to having no types whatsoever. not a fan of dyncamic typing and so is my editor apparently =)

image

list comprehensions are largely unique to python, writing list(map(lambda t: t.text, tags)) is characteristic for someone new to the language in general not necessarily js devs.

Willy-JL commented 10 months ago

well done, looks good overall. i tweaked a few things code wise, just how i would write them personally.

however, instead of storing 3 different lists, i believe it would be more appropriate to have an enum for TagHighlight, and then keep the preference in a dict that maps tag id to a taghighlight value. think of it in more higher level terms, each tag can only be one of 4 states, it cant be highlighted in multiple ways at the same time. thats also what i wanted to do with the dynamic tag database, this state couldve been tied to the generic tag info thats scraped from the forum, but thats not possible. all this is not to say yours is a bad implementation, just that it could be a little cleaner imo. ill tweak that real quick

as for the popup to customize, i had a different idea personally. the tabs idea and only showing the highlighted ones makes sense if its implemented with lists under the hood, its natural. but i think that showing all tags one after the other, and allowing to click them to cycle between normal, positive, negative, critical would make more sense. also means you can reuse this and allow changing the preference anytime the tag is shown, like in a game info popup