EurekaCommunity / SplitRow

A row for Eureka to put two rows side by side into the same UITableViewCell
MIT License
56 stars 27 forks source link

Wrong background color in iOS13 darkmode #41

Closed leafarenuk closed 4 years ago

leafarenuk commented 4 years ago

HI there,

I just updated to version 2.1.0 but I still got a strange issue:

When the dark mode is activated I get a wrong background color for Rows which are embedded in the Splitrow (see Screenshot)

Is this working as expected or really an issue? How can I change the background color so that it matches that of the "Feedback button"?

Thank you in advance!

CleanShot2019-11-05at21 02 13

My code setup looks like this:

<<< SplitRow<ButtonRow, ButtonRow>(){
                $0.rowLeftPercentage = 0.5
                $0.rowLeft = ButtonRow() {
                    $0.title = "Show Intro"
                    $0.onCellSelection(self.openIntro)
                }
                $0.rowRight = ButtonRow() {
                    $0.title = "Whats new?"
                    $0.onCellSelection(self.openWhatsNew)
                }
            }

            <<< ButtonRow() {
                $0.title = "Feedback"
                $0.onCellSelection (self.sendMail)
            }
marbetschar commented 4 years ago

Not quite sure if this really is an issue with SplitRow itself. Dark Mode knows different "layers" of content and reflects these layers in different shades of gray.

You might want to experiment a bit with the possibilities using the available content background colors:

To set these colors you can use the }.cellSetup {, and/or }.cellUpdate { on the row itself - or, if you want to implement it globally for a specific type of cell you should also be able to use the SplitRow<ButtonRow, ButtonRow>.defaultCellSetup and/or SplitRow<ButtonRow, ButtonRow>.defaultCellUpdate

For further information see:

I'll close this issue for now. Feel free to reopen if you feel this really is a bug with SplitRow.

kamerc commented 4 years ago

I am seeing something similar in my app too. I spent a long time trying to debug this a few months ago and honestly couldn't find anywhere that SplitRow was doing anything custom with background color. The code change that was just pushed is only to use Eureka's latest version with dark mode support. I didn't change any code anywhere with SplitRow.

Screen Shot 2019-11-05 at 3 38 19 PM

leafarenuk commented 4 years ago

@kamerc thank you for your comment! Sorry maybe my comment was a little misunderstanding:

This problem occurred also before the update to 2.1.0...

I spent a long time trying to debug this a few months ago and honestly couldn't find anywhere that SplitRow was doing anything custom with background color. -> that sounds like I also have to put some effort into it 🤷🏼‍♂️ I will let you know if I can solve it!

leafarenuk commented 4 years ago

@kamerc thank you for your comment! Sorry maybe my comment was a little misunderstanding:

This problem occurred also before the update to 2.1.0...

I spent a long time trying to debug this a few months ago and honestly couldn't find anywhere that SplitRow was doing anything custom with background color. -> that sounds like I also have to put some effort into it 🤷🏼‍♂️ I will let you know if I can solve it!

@kamerc I resolved it with the following:

.cellSetup({ (cell, row) in
                    if #available(iOS 13.0, *) {
                        cell.backgroundColor = .clear
                    } else {
                        // Fallback on earlier versions
                    }
                })

.clear was the correct color 🎉🎉🎉

kamerc commented 4 years ago

@leafarenuk I had also tried to set the background color to .clear and didn't see a difference. However, my code was actually within the SplitRow codebase. If it works with cellSetup then perhaps I just didn't put the logic in the correct place in SplitRow. I do think this logic you have above needs to be done within the SplitRow code but I'm not sure exactly where. I am going to re-open it for that reason.

leafarenuk commented 4 years ago

I have put .cellSetup() directly after the code for creating the rowLeft and rowRight:

            <<< SplitRow<ButtonRow, ButtonRow>(){
                $0.rowLeftPercentage = 0.5
                $0.rowLeft = ButtonRow() {
                    $0.title = "Show Intro"
                    $0.onCellSelection(self.openIntro)
                }.cellSetup({ (cell, row) in
                    if #available(iOS 13.0, *) {
                        cell.backgroundColor = .clear
                    } else {
                        // Fallback on earlier versions
                    }
                })

                $0.rowRight = ButtonRow() {
                    $0.title = "Whats new?"
                    $0.onCellSelection(self.openWhatsNew)
                }.cellSetup({ (cell, row) in
                    if #available(iOS 13.0, *) {
                        cell.backgroundColor = .clear
                    } else {
                        // Fallback on earlier versions
                    }
                })
            }
kamerc commented 4 years ago

@leafarenuk I meant where within the SplitRow code not where within the customizations that can be made with an instance of SplitRow. Hope that makes sense. Everyone who uses SplitRow shouldn't have to go add this customization logic to their code for it to work right in dark mode.

marbetschar commented 4 years ago

Eureka expects UITableViewCells and relies on UITableView events, SplitRow simply creates two embedded UITableViews and places them next to each other within the parent UITableView. That's where SplitRowCellTableView comes into play.

That said, I think we can add the provided fix to the SplitRowCellTableView class - in the setup() method.

I'll add this fix to the master branch. @leafarenuk are you using Carthage? If so, you'll be able to checkout the master branch easily to test if the changes do have the desired effect.

marbetschar commented 4 years ago

@leafarenuk if you're using Carthage, please add the following to your Cartfile to test the provided fix:

github "EurekaCommunity/SplitRow" "master"

... and run carthage update --platform iOS to update SplitRow.

leafarenuk commented 4 years ago

@leafarenuk if you're using Carthage, please add the following to your Cartfile to test the provided fix:

github "EurekaCommunity/SplitRow" "master"

... and run carthage update --platform iOS to update SplitRow.

I am using Cocoapods, maybe @kamerc can try it?

Thanks for your support @marbetschar !!

marbetschar commented 4 years ago

Well, then lets do the release dance one more time. From what I can tell, the issue is not present in the Example app and things work as expected. So we're good to go for v2.1.1...

marbetschar commented 4 years ago

@leafarenuk @kamerc v2.1.1 is available through Cocoapods. Can you verify the provided fix works as expected?

leafarenuk commented 4 years ago

works fine for me. I removed my .cellSetup() from my initialization and the background is displayed in the correct color. Thanks again, for me this can be closed!

kamerc commented 4 years ago

Sorry for the delayed reply, I can confirm that 2.1.1 fixes my issue