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

Fixes for custom buttons #837

Merged
merged 2 commits into from
Jan 19, 2023
Merged

Conversation

wutschel
Copy link
Collaborator

Description

As mentioned in #836 there is still a problem with the layout after sleep / resume cell in RightMenuViewController. The former code also was not using dequeueing but loaded the cell fresh each time. To unblock the release 1.12 I propose a workaround which again loads the custom button rows each time when entering cellForRowAtIndexPath.

This needs another rework in future.

Summary for release notes

Bugfix: Improve readability of button rename icon in light mode
Bugfix: Fix custom button layout problems after sleep / resume

@wutschel
Copy link
Collaborator Author

First option is to simply not dequeue and always load fresh cell.

Second option is to only do refresh for the custom buttons (as done in the old code before unifying the nib-loading). Due to refactoring this now adds more code. In case we want to keep this, I can move the doubled up variable initialization into a helper method.

@kambala-decapitator
Copy link
Collaborator

I guess second option is the last commit. Let's go with the first option then (the old way, if I get it right).

@wutschel
Copy link
Collaborator Author

The "old way" was more like the 2nd option (last commit). But the simpler workaround is 1st option, which I personally prefer. If you agree, I will just remove the last commit.

I'll keep #836 open as I would need your help to find a proper solution. We can discuss it there.

@kambala-decapitator kambala-decapitator merged commit 2204f43 into xbmc:master Jan 19, 2023
@wutschel wutschel deleted the fix_custom_button branch January 19, 2023 17:58
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.

2 participants