search

icykoneko / ff14-fish-tracker-app

FF14 Fish Tracker App
https://ff14fish.carbuncleplushy.com
MIT License
61 stars 21 forks source link

(fixes #150) detect inituition fish availability changes #153

Closed blandcr closed 5 months ago

blandcr commented 5 months ago

When the layout updates subEntries (intuition fish) it does not capture whether the subEntry availability has changed. This means intuition fish availability changes don't provoke a layout sort. Without a layout sort the "fish-availability-transitioning" class won't get removed, so the text for the time remaining remains set of 0 opacity.

This PR "hoists" availability changes from intuition fish up into the parent fish entry which should provoke a call to this.layout.sort from the view model. I'm not sure if this is the best fix but it seems to work in the dev environment.

Since we don't want to force an expensive sort, and the fish-availability-transitioning class was not functioning correctly this PR removes the class and associated logic and style(s) to fix the issue (#150) with intuition fish countdowns disappearing.

icykoneko commented 5 months ago

This might fix it, but looked back to where that class was added, and I think the problem is actually more to do with adding the class to an intuition row.

Resorting the display is very costly the more fish the user has showing, so I really try to avoid doing it as much as possible.

Doing some more digging... I don't even think that fish-availability-transitioning class is even helpful. I paused the script as it ran update, but the text had already changed, which originally was the reason I think I added this class.

Take a look at 659cb5a8ef34f3d3d2a7fd64987c9b2301e73a19. I made other changes to the the timers but I guess I never reverted adding this class. Can you see if just removing the class altogether solves the issue, and doesn't result in more display bugs.

blandcr commented 5 months ago

Yeah, I agree fish-availability-transitioning does not seem to do anything. I guess the easiest solution is just to remove it for the time being. Let me know if you want me to update this PR in that way. On another note I did notice seems to be some desync between when when style changes get painted and when other DOM changes even happen as I can see the "active" background color on newly inactive fishes for a brief moment after the sorting has happened, however the fish-active class already should have been removed by that point. However, I'm still learning about how all this javascript and web stuff works so I'm not really sure.

icykoneko commented 5 months ago

I noticed this as well. It’s definitely due to sorting happening later. But style rules get applied all at once like animation frames. For now, it’s fine.

Go ahead and remove the class and I’ll merge the PR. I appreciate the help.

On Thu, Jan 25, 2024 at 11:36 PM C. Bland @.***> wrote:

Yeah, I agree fish-availability-transitioning does not seem to do anything. I guess the easiest solution is just to remove it for the time being. Let me know if you want me to update this PR in that way. On another note I did notice seems to be some desync between when when style changes get painted and when other DOM changes even happen as I can see the "active" background color on newly inactive fishes for a brief moment after the sorting has happened, however the fish-active class already should have been removed by that point. However, I'm still learning about how all this javascript and web stuff works so I'm not really sure.

— Reply to this email directly, view it on GitHub https://github.com/icykoneko/ff14-fish-tracker-app/pull/153#issuecomment-1911467401, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG2BPIX5CWXP3BZJ7P5IOP3YQMXF7AVCNFSM6AAAAABCJHFZVSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJRGQ3DONBQGE . You are receiving this because you commented.Message ID: @.***>

blandcr commented 5 months ago

Sure thing, PR is updated with the new changes. Fish tracker is a great tool so I'm happy to help!