Skip to content
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.

Sign up for GitHub

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

Jump to bottom

FIX: Correctly style details deaths when using gradient theme. #36

Merged
merged 1 commit into from
Jan 31, 2023
Merged

FIX: Correctly style details deaths when using gradient theme. #36

merged 1 commit into from
Jan 31, 2023

Conversation

shrift
Copy link
Contributor

@shrift shrift commented Jan 29, 2023 •
edited
Loading

Summary of Changes

  1. Resolves an issue with details gradient theme where bars were sometimes not correctly styled.

Description

Some details report types have bars that do not have an actor() method implemented.
In these cases the required class id is stored in a table on an unnamed element at index 4.
This seems fragile, but there is also not any other clear way to access the requisite class id using only what is passed to the existing GetRowColor method.

To test

  • Checkout the changes in the PR
  • Load the game and change a details window to Deaths
  • Die
  • Observe gradient should now display correctly

@shrift
Copy link
Contributor Author

shrift commented Jan 29, 2023

I'm new to WoW addon development, so I took a light approach here.
I think this fix may be fragile: if the element index changes in the future this will break again.
I'm not familiar enough to know if this a common risk in addon development.

A more robust would be to use some details API I'm not aware of, or to get the class color through some other means.

I think this fix is at least not any worse than the current implementation which is known to be broken; if the change in this PR were to break it would be in a similar way to the current state.

@shrift shrift force-pushed the fix_gradient_details_deaths branch from e1d73b9 to c4553e7 Compare January 29, 2023 23:15
@RyadaProductions
Copy link
Collaborator

I tested this tonight, and it does solve the lua errors we normally would get in this display.
The only downside is that the images of the class are still not being set correctly, would you want to fix that as well or would you want this merged and then somebody else or a different PR for the images?
As an example of what I mean:
image

@shrift
Copy link
Contributor Author

shrift commented Jan 30, 2023

I also noticed that, but didn't actually know for sure that it was wrong behavior as this has been broken the whole time I've used ToxiUI; I wasn't sure which icons were supposed to be there. Is it just the class icon?

If we're good with the change so far, let's merge it as is and I'll follow up with a fix for the icons.
I think there is good value in eliminating the lua error, so want to get that out there in case the fix for the icons takes a while to figure out.

@Toxicom Toxicom merged commit 0f44f72 into Toxicom:development Jan 31, 2023
Toxicom added a commit that referenced this pull request Mar 7, 2023
@Toxicom @Wolen
@Uncaught3xception @bagelbits @RyadaProductions @shrift
🚀 RELEASE: 6.1.6 (#45)
cac0540 
* Fixed Rune Abbreviation (#4)

* 👌 IMPROVE: Match default UI action bars (#5)

* 👌 IMPROVE: Match default UI action bars

* 🤖 TEST: Fix CI

* 9.2.7 -> 10.0 Fixes (#6)

* Fixes a lot of the errors to get it running.

* Style fix

* 🤖 TEST: Remove unused vars

* 🤖 TEST: Follow-up

* 🤖 TEST: Remove `.bak` files

* WeakAuraAnchor fix

Co-authored-by: Tomas Leonavičius <69549795+Toxicom@users.noreply.github.com>

* 👌 IMPROVE: Fuck off taints

* 📖 DOC: Prepare badge for Wolen

* 👌 IMPROVE: Disable bars 13-15

* 🐛 FIX: Update WAAnchor position

* 🤖 TEST: umm maybe fix CI

* 📖 DOC: Update changelog

* 📖 DOC: Update DF WA link

* 🐛 FIX: Fixed Rune Enchant Abbr (#7)

* 🐛 FIX: Fixed Profession Open / Close (#10)

* 🐛 FIX: Revert font outline change

* 🤖 TEST: ci fix

* 🤖 TEST: Remove unused var

* 🤖 TEST: Personal font changes

* 🐛 FIX: Wunderbar Professions, VehicleBar (#11)

* Fixed Wunderbar Professions, VehicleBar

* 🤖 TEST: Update luacheckrc

* 🐛 FIX: More Vehicle Fixes

Co-authored-by: Tomas Leonavičius <69549795+Toxicom@users.noreply.github.com>

* 📖 DOC: Clean up badges

* 🐛 FIX: Reputation bar is no longer 100% (#13)

* 📖 DOC: Ande chat badge

* #12 DataBar & professions fix

* 🤖 TEST: ci fix

* Various fixes (#14)

* 🐛 FIX: ElvUI Theme

* 🐛 FIX: Update WB Currency module

* 🐛 FIX: Update WA Anchor

* 🐛 FIX: Update BlizzardFonts

* 🐛 FIX: WB Professions updates

* 🤖 TEST: CI fix?

* Update README.md

* Update README.md

* 🤖 TEST: Disable custom buttons again

* 🐛 FIX: Clicking hearthstone works again (#15)

* 🐛 FIX: Reverted Vehiclebar Updating/Paging (#16)

* ElvUI update (#17)

* 🐛 FIX: ElvUI 13.01 changes

* 🐛 FIX: Lib Compress/Base64 removal

* 📖 DOC: Update TOC

* 10.02 (#19)

* 🐛 FIX: 10.02 Fixes

* 🐛 FIX: Durability fix

* 🐛 FIX: Tooltip sticking around

* 📖 DOC: Update TOC

* 🤖 TEST: Remove unused vars

* 🐛 FIX: CI Fix

Co-authored-by: Wolen <wolen2102@gmail.com>

* 🐛 FIX: Added Retail checks for Classic functions (#20)

* 🐛 FIX: Added Retail checks for Classic functions

* 🤖 TEST: CI

* 🤖 TEST: For real this time

* 📖 DOC: dragons am i right

* 📖 DOC: Update evoker colors

* 🤖 TEST: personal micromenu chng

* 📖 DOC: Update evoker spec icons

* 📖 DOC: Make WindTools required

* 📖 DOC: Bump ElvUI req. version

* 📖 DOC: Update chat badges

* 🤖 TEST: personal WA change

* ‼️ BREAKING: Maybe fix WA skin windtools thingy?

* 🤖 TEST:Disable force gradient mode during install

* 🐛 FIX: Fix healer party buffs anchor

* ‼️ BREAKING: follow-up to WA skin "fix"

* 👌 IMPROVE: Apply nawu's tag patch

* 📖 DOC: Bump TOC for elvui req. vers

* 🐛 FIX: Fix secure flyouts

* 📖 DOC: Update changelog

* 📖 DOC: Add github to contacts (need logo)

* 🚀 RELEASE: 6.1.0

* 📖 DOC: Prepare TOC for 6.1.2

* 👌 IMPROVE: Update plater mods

* 📖 DOC: CL update

* 🤖 TEST: personal wb chng

* 🐛 FIX: Fix gradient dead color?

* 👌 IMPROVE: Move vigor bar down

* 👌 IMPROVE: Remove talking head mover

* 👌 IMPROVE: Remove totem bar stuff

* 👌 IMPROVE: Update rogue gradient color

* 🤖 TEST: toxi personal

* 📦 NEW: Add Evoker spec icons to WB & Armory

* 👌 IMPROVE: Add valdrakken portals for mages

* 📦 NEW: Change default WB background

* 👌 IMPROVE: Add tol barad portals for mages

* 🤖 TEST: toxi personal wb

* 👌 IMPROVE: Minimap hide tracking & upscale LFG

* 📖 DOC: Organize couple changelogs a bit

* 🤖 TEST: Fix CL LUA error

* 🤖 TEST: Toxi personal WT change

* 🐛 FIX: maybe fix splashscreen error

* Dragonriding (#22)

* 📦 NEW: Dragonriding Vehiclebar toggle

* 🤖 TEST: Fixed extra bonusbar

* DOC: Update v0dka names

* Support for the dragonflight enchant/socket system (#23)

* 👌 IMPROVE: Dragonflight enchantments support

* 📦 NEW: Optional missing socket support on necklaces

* 📖 DOC: Changelog

* 🤖 TEST: Update toxi char names

* 📦 NEW: missing sockets and enchants code (#24)

* 👌 IMPROVE: missing sockets and enchants code

* 📖 DOC: Update checkbox descriptions

* 👌 IMPROVE: Small tweaks to FastColorGradientHex

* 📖 DOC: Changelog update

* 📖 DOC: Enable missing sockets by default

* 👌 IMPROVE: Rename `messageCondition`

Co-authored-by: Tomas Leonavičius <69549795+Toxicom@users.noreply.github.com>

* 🤖 TEST: personal

* 🤖 TEST: Change wb bg personal

* 👌 IMPROVE: Focus Frame reposition (#25)

* 👌 IMPROVE: Update Plater mods

* 🐛 FIX: Vehicle bar taint (#26)

* 📖 DOC: Changelog

* 📖 DOC: Init 6.1.3

* 👌 IMPROVE: Disable plater target color

* 🐛 FIX: Disable plater coloring for rares

* 👌 IMPROVE: Force disable ElvUI UW support

* 👌 IMPROVE: Remove gigachad resolution calc

* 📖 DOC: CL Update

* 👌 IMPROVE: Update ElvUI Globals

-   Move World Map coordinates to bottom right;
-   Increase default elvui options window size.

* 📖 DOC: Bump required elvui version

* 🤖 TEST: Personal bag font change

* 🤖 TEST: Test discord webhook

* 📦 NEW: Add Hoffi badge names

* 🐛 FIX: Raid groups' visiblity state

* 🤖 TEST: personal toxi

* 🤖 TEST: personal dk color

* 📦 NEW: Title Secondary font

* 📖 DOC: Update font description

* 📖 DOC: Bump required ElvUI version

* 🐛 FIX: Wrath Ulduar API changes

* 📖 DOC: Update changelog

* 🤖 TEST: luacheck

* 📖 DOC: Init 6.1.4

* 🐛 FIX: UI Scale for 2160p

* 🐛 FIX: Debug messages in stable

* 📖 DOC: Init 6.1.5

* 📖 DOC: Reduce log level for dev release

* 📖 DOC: Bump TOC

* 🐛 FIX: Disable guild tooltip and rightclick

* 📖 DOC: Init 6.1.6

* FIX: Correctly style details deaths when using gradient theme. (#36)

* 📖 DOC: Update changelog

* #40 Installer changes (#42)

---------

Co-authored-by: Wolen <wolen2102@gmail.com>
Co-authored-by: Uncaught3xception <67075781+Uncaught3xception@users.noreply.github.com>
Co-authored-by: Chris Ward <cdurianward@gmail.com>
Co-authored-by: Rochelle <RyadaProductions@users.noreply.github.com>
Co-authored-by: Brendan Martens <shrift@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@shrift @RyadaProductions @Toxicom