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 an issue where byAdding(.extraAttributes(_:)) removes previously assigned extraAttributes. #430

Conversation

stleamist
Copy link
Contributor

@stleamist stleamist commented Jun 9, 2023

Issue

byAdding(stringStyle:) preserves previously assigned extraAttributes, but byAdding(_:) doesn't.

let style = StringStyle()
    .byAdding(stringStyle: StringStyle(.extraAttributes([.backgroundColor: UIColor.white])))
    .byAdding(stringStyle: StringStyle(.extraAttributes([.foregroundColor: UIColor.black])))

BONAssert(attributes: style.attributes, key: .backgroundColor, value: UIColor.white) // ✅
BONAssert(attributes: style.attributes, key: .foregroundColor, value: UIColor.black) // ✅
let style = StringStyle()
    .byAdding(.extraAttributes([.backgroundColor: UIColor.white]))
    .byAdding(.extraAttributes([.foregroundColor: UIColor.black])) // overwrites previous line

BONAssert(attributes: style.attributes, key: .backgroundColor, value: UIColor.white) // ❌
BONAssert(attributes: style.attributes, key: .foregroundColor, value: UIColor.black) // ✅

Solution

Fixed update(part:) method to use add(extraAttributes:) for .extraAttributes(_:), like byAdding(stringStyle:) does.

@ZevEisenberg
Copy link
Collaborator

Thank you! This is a great catch. Would you mind swapping UIColor for BONColor, a platform-agnostic alias that will allow the tests to run on macOS?

@stleamist
Copy link
Contributor Author

Okay! I’ll swap it right now :)

@stleamist stleamist force-pushed the bug/byadding-parts-extraattributes-overriding branch from 6c79a4c to 90e216b Compare June 9, 2023 13:48
@stleamist
Copy link
Contributor Author

Hi @ZevEisenberg,

Could you review this PR, please?

@ZevEisenberg ZevEisenberg merged commit becf396 into Rightpoint:master Jun 12, 2023
@ZevEisenberg
Copy link
Collaborator

Merged and released! Thanks for your contribution. It makes me happy to know BonMot has active users!

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