search

alorma / Compose-Settings

Compose Multiplatform #Compose Settings library
https://alorma.github.io/Compose-Settings/
MIT License
428 stars 27 forks source link

Add enabled to SettingsGroup & Fix padding in Sample #267

Closed TinyHai closed 4 months ago

TinyHai commented 5 months ago

I added enabled to the SettingsGroup. Now Settings will get the default enabled value from the Group which directly wraps it.

I fixed the padding in Sample as I was writing samples.

TinyHai commented 4 months ago

As you fixed some paddings, could you provide a before / after comparative so I understand what was wrong?

Actually, I don't add any padding. I just apply the padding Scaffold provided to its content, so that its content will not be covered by SampleTopBar. Review App.kt

Do you remember this? the SettingsSwitch was covered by SampleTopBar. My work is to rescue the poor SettingsSwitch

TinyHai commented 4 months ago

I notice that the job Validate screenshots is failling.

Should I push the screenshots that generated by executing ./gradlew updateScreenshotTest?

alorma commented 4 months ago

No, don't worry.

New screenshots will fail, it's ok. As screenshots are generated on GHA those will fail if generated on your local.

I assume that, I know those will fail when added on PRs.

Let me review this PR and merge with admin power later :)

alorma commented 4 months ago

Looks great! Thanks for your contribution!

alorma commented 4 months ago

BTW, can you updadte README.md to reflect the enabled, change on group?

TinyHai commented 4 months ago

BTW, can you updadte README.md to reflect the enabled, change on group?

no problem