Tanza3D / reddark

reddark, but it's in realtime
GNU Affero General Public License v3.0
298 stars 41 forks source link

added check for restricted sub in response json #39

Open YoloFTW opened 1 year ago

YoloFTW commented 1 year ago

Added a check for restricted subs.

Also added some css if the colour of these subs would be different from private subs. currently the same as private

Tanza3D commented 1 year ago

ah, awesome, didn't know you could do this! will review in just a minute

Tanza3D commented 1 year ago

image not sure if this is an issue with me but it seems to cause this spam

YoloFTW commented 1 year ago

Hmmm strange i didnt encounter spam. I'll look into this

lokzz commented 1 year ago

btw this also causes the "dark'd" subreddit count to go down as it isnt in the "private" status type not good. image

YoloFTW commented 1 year ago

I believe the issue with the notifications is because i neglected to check if it was already restricted and it should be fixed now.

I also changed the counter to count restricted as dark so that should be fixed too.

If these are still broken let me know and ill look at it again

Tanza3D commented 1 year ago

might there be an error in the code somewhere? i can never seem to finish a loop (gets caught at the try { } catch { } block)

YoloFTW commented 1 year ago

does it run the catch block or does it just loop endlessly without output?

YoloFTW commented 1 year ago

it looks to be a rate limit issue. if reddit blocks the last requests on the first pass no data is ever sent to the frontend and the program never loops itself

username-is-required commented 1 year ago

you’ll probably also want to change the line on index.html that says that restricted subreddits will be displayed as public

https://github.com/YoloFTW/reddark/blob/3e1719093bda8fd234a58bc67a643aa429d1299e/index.html#L42-L43

username-is-required commented 1 year ago

genItem() in public/index.js also needs updating to deal with restricted subs

https://github.com/YoloFTW/reddark/blob/3e1719093bda8fd234a58bc67a643aa429d1299e/public/index.js#L126-L128

(sorry for the comment spam! i’m just commenting these as i find them while implementing this into my fork)

YoloFTW commented 1 year ago

No problem!

Ive implemented all of your suggestions and it all apears to be working

As always if you find something wrong let me know and ill try to fix it

Screenshot 2023-06-12 124910

username-is-required commented 1 year ago

@YoloFTW whats your opinion on whether the colour should be different for restricted subs than that of private subs? i had one person tell me they thought restricted subs should be gold (i think i’ve heard that somewhere else as well but i can’t remember where) so curious for your opinion on it

YoloFTW commented 1 year ago

I can see why some subreddits might want to go restricted instead of private so it makes sence to me to keep them the same colour, however I can see the benifit of differentiating them from a glance.

I created a speperate class for the restricted subs in index.css so if people wanted to have them a specific colour they could.

All that being said I think keeping them green is fine however im not opposed to them being gold or some other colour.

Maybe we could add some sort of options on the front end so users can configure the colours but its just a thought.

OdinintheNorth commented 1 year ago

Not sure if it's ok to chime in as someone not contributing code, but I'd definitely vote for restricted subs being gold to help distinguish them

YoloFTW commented 1 year ago

@OverlordOdin After looking back I agree so I made a quick change to make restricted subs gold.

Screenshot 2023-06-12 192644

OdinintheNorth commented 1 year ago

Looks great! Hopefully this can get merged soon

YoloFTW commented 1 year ago

@Tanza3D dropping a quick note to inform you that I've resolved the issues raised. If you could spare a moment to review my updated pull request, I'd be grateful. Thank you!

username-is-required commented 1 year ago

the code here also needs to remove the subreddit-restricted class

username-is-required commented 1 year ago

for here and here, the code needs to check if the subreddit is already private/restricted and, if so, remove the other class

YoloFTW commented 1 year ago

My bad. ill fix these now and get it pushed up.

YoloFTW commented 1 year ago

Ive pushed the changes and should be all up to date and ready to merge.

As always let me know if theres anything else