kakaroto / Beyond20

D&D Beyond Character Sheet Integration in Roll20
GNU General Public License v3.0
484 stars 141 forks source link

Fix - issue with action rolls from DDB changes #1133

Closed Azmoria closed 1 month ago

Azmoria commented 1 month ago

This change should resolve the action rolling issues from DDB changes today. Looks like they changed the sidebar info classes.

jtbg commented 1 month ago

Tested and confirmed that this change fixes #1131, #1132, and #1134

Kodokbotak commented 1 month ago

how do you apply this fix?

stonesteel25 commented 1 month ago

I have tried this with the developer version and it doesn't resolve it on my end

jtbg commented 1 month ago

I have tried this with the developer version and it doesn't resolve it on my end

@stonesteel25 any chance you could you post the crx/xpi package of your build?

how do you apply this fix?

@Kodokbotak after code review, it will be merged and the extension will be updated on the Chrome/firefox/etc store. If you desperately need a fix in the meantime, join the discord and I can share a dev build for you

dmportella commented 1 month ago

that worked for me

I would change or remove the if statement

if we want to keep the if statement in propertyListToDict adding toLowerCase() to the filter also fixes the problem.

        // April 2024 website redesign now uses dynamic class names with styled components
        if (labelElement.length === 0 || valueElement.length === 0) {
            labelElement = prop.children().filter((i, el) => el.className.toLowerCase().includes("label"));
            valueElement = prop.children().filter((i, el) => el.className.toLowerCase().includes("value"));
        }

Both changes fix the problem.

kakaroto commented 1 month ago

that worked for me

I would change or remove the if statement

if we want to keep the if statement in propertyListToDict adding toLowerCase() to the filter also fixes the problem.

        // April 2024 website redesign now uses dynamic class names with styled components
        if (labelElement.length === 0 || valueElement.length === 0) {
            labelElement = prop.children().filter((i, el) => el.className.toLowerCase().includes("label"));
            valueElement = prop.children().filter((i, el) => el.className.toLowerCase().includes("value"));
        }

Both changes fix the problem.

Why did they need to change the class name from Label to label ? 😭 Thanks for the fix, I'll merge this one, and also apply the change to the lowercase label/value class names

dmportella commented 1 month ago

Why did they need to change the class name from Label to label ? 😭 Thanks for the fix, I'll merge this one, and also apply the change to the lowercase label/value class names

I know T, T silly changes not sure why they did that for, what worries me most was that they added styled components a terrible module - where I work we are spending $$$$$ to remove it