Fix application of modifiers in Markdown component#1893
Conversation
JayShortway
left a comment
There was a problem hiding this comment.
This is great, thanks for fixing this! I think this actually fixes a bunch of violations in our detekt-baseline.xml. Could you regenerate that?
Hmm seems the changes to the baseline weren't related (just pushed them in any case)... Probably could try to fix some others, but seems like something unrelated for this PR... But yeah, we should probably try to fix those when we can |
Markdown component
📸 Snapshot Test43 modified, 31 unchanged
🛸 Powered by Emerge Tools |
|
This has changed the snapshots a bit as can be seen in Emerge ^... Mostly some margins have moved... I feel these are ok but would like someone to double-check @JayShortway @vegaro |
|
Regarding the baseline: that's interesting. Seems like this specific one wasn't caught by Detekt. Regarding the Emerge screenshots: my only concern is the |
Hmm good call... Going to see if I can minimize the change there. Great catch! |
| allowLinks = false, | ||
| modifier = Modifier | ||
| .padding(vertical = UIConstant.defaultVerticalSpacing / 3) | ||
| .padding(vertical = UIConstant.defaultVerticalSpacing * 2 / 3) |
There was a problem hiding this comment.
This doesn't exactly match back up to the original size but it's pretty close. I think it's difficult to match it exactly since the components within markdown before perform some custom painting that was altering the sizes slightly... Lmk what you think!
There was a problem hiding this comment.
We could also use a minimum height instead of relying on the font size + padding, but not a blocker for me.
JayShortway
left a comment
There was a problem hiding this comment.
Very nice improvement! Check out this one, the new one is much better haha. Thanks for fixing!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1893 +/- ##
=======================================
Coverage 82.25% 82.25%
=======================================
Files 227 227
Lines 7968 7968
Branches 1121 1121
=======================================
Hits 6554 6554
Misses 967 967
Partials 447 447 ☔ View full report in Codecov by Sentry. |
**This is an automatic release.** ## RevenueCat SDK ### ✨ New Features * [Experimental] Web purchase redemption (#1889) via Toni Rico (@tonidero) ### 🐞 Bugfixes * Keeps the org.json package. (#1891) via JayShortway (@JayShortway) ### 📦 Dependency Updates * Bump rexml from 3.3.8 to 3.3.9 (#1892) via dependabot[bot] (@dependabot[bot]) * Bump danger from 9.5.0 to 9.5.1 (#1885) via dependabot[bot] (@dependabot[bot]) * Bump fastlane from 2.224.0 to 2.225.0 (#1884) via dependabot[bot] (@dependabot[bot]) ## RevenueCatUI SDK ### 🐞 Bugfixes * Fix application of modifiers in `Markdown` component (#1893) via Toni Rico (@tonidero) ### 🔄 Other Changes * [CI] Allow publishing snapshot releases manually from branches (#1888) via Toni Rico (@tonidero) * Detekt auto-fixes (#1886) via Toni Rico (@tonidero) Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Description
We were passing the modifier for the
Markdowncomposable down to the whole composable tree inside that Markdown. We are currently passing some margins to the markdown composable which were basically applied to all individual components of the markdown, so if processing the markdown generated several components, all of them were applied the paddings for example.I believe this was not expected so I decided to just not propagate the modifiers inside the
Markdowncomposable and just keep it at the root.