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.

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

Proposed Keyring Support Changes #590

Draft
wants to merge 1 commit into
base: bcc
Choose a base branch
from
Draft

Proposed Keyring Support Changes #590

wants to merge 1 commit into from

Conversation

Talyrius
Copy link
Contributor

@Talyrius Talyrius commented Jun 19, 2021

AdiBags is a complicated addon and I'm fairly busy, so I don't have a whole lot of time to dedicate to fully fleshing this out right now. If you feel so inclined, please take a look at these proposed changes to the initial keyring support (adb91fe) added for Burning Crusade Classic and offer a hand.

Update/clarification:
Keyring support (adb91fe) didn't survive the unification of the codebase (#648).

@Talyrius Talyrius marked this pull request as draft June 19, 2021 23:14
@Talyrius Talyrius requested a review from Adirelle June 19, 2021 23:21
@Ragnoroct
Copy link

I think this solution just reverts the keyring changes right? Is that what we want? I thought it was to just make the free key slots not show up in the free space.

@Ragnoroct
Copy link

This is the solution I came up with. It hides both the Key section and the Key slots in the Free space section when you toggle it's visibility.

From 532fc829183768900cb860cb4e2580784858a7e1 Mon Sep 17 00:00:00 2001
From: Will Bender <[email protected]>
Date: Tue, 3 May 2022 22:57:40 -1000
Subject: [PATCH] What I think works

---
 widgets/Section.lua | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/widgets/Section.lua b/widgets/Section.lua
index 7b020d3..a43df9a 100644
--- a/widgets/Section.lua
+++ b/widgets/Section.lua
@@ -108,8 +108,15 @@ function sectionProto:OnCreate()
 end
 
 function sectionProto:OnShow()
+	local isFreeSpaceSection = self.key == "Free space#Free space"
+	local keySection = self.container:GetSection("Key", "Key")
+
 	for button in pairs(self.buttons) do
-		button:Show()
+		local buttonFamilyIsKeyChain = button.bagFamily == 256 or (button:IsStack() and button:GetBagFamily() == 256)
+		local shouldShow = not (keySection:IsCollapsed() and isFreeSpaceSection and buttonFamilyIsKeyChain)
+		if shouldShow then
+			button:Show()
+		end
 	end
 end
 
@@ -117,6 +124,15 @@ function sectionProto:OnHide()
 	for button in pairs(self.buttons) do
 		button:Hide()
 	end
+	-- Also hide free keys section in the "Free space"
+	if self.key == "Key#Key" then
+		local section = self.container:GetSection("Free space", "Free space")
+		for button in pairs(section.buttons) do
+			if button.bagFamily == 256 or (button:IsStack() and button:GetBagFamily() == 256) then
+				button:Hide()
+			end
+		end
+	end
 end
 
 function sectionProto:ToString()
@@ -358,9 +374,16 @@ function sectionProto:FullLayout()
 		return self:Hide()
 	end
 
+	local isFreeSpaceSection = self.key == "Free space#Free space"
+	local keySection = self.container:GetSection("Key", "Key")
+
 	for button in pairs(self.buttons) do
-		button:Show()
-		tinsert(buttonOrder, button)
+		local buttonFamilyIsKeyChain = button.bagFamily == 256 or (button:IsStack() and button:GetBagFamily() == 256)
+		local shouldShow = not (keySection:IsCollapsed() and isFreeSpaceSection and buttonFamilyIsKeyChain)
+		if shouldShow then
+			button:Show()
+			tinsert(buttonOrder, button)
+		end
 	end
 	tsort(buttonOrder, CompareButtons)
 
-- 
2.36.0.windows.1

@Talyrius
Copy link
Contributor Author

Talyrius commented May 4, 2022

Thanks for sharing your solution. I'll review it when I get around to merging the classic/bcc branches back into master (soon, hopefully).

@jazminite
Copy link

Any update on merging in this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to disable keyring module that was introduced in latest tbc update?
4 participants