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

Advanced styling of headings #992

Merged
merged 15 commits into from
Jul 19, 2022
Merged

Advanced styling of headings #992

merged 15 commits into from
Jul 19, 2022

Conversation

planarvoid
Copy link
Contributor

@planarvoid planarvoid commented Jul 8, 2022

Fix

In this PR I'm introducing advanced options for styling some elements. For example headings now can be sized and changed each one differently. Headings now support size and color.

Test

  1. Modify the AztecText style in MainActivity to use headingNFontSize or headingNFontColor where N stands for heading types between one and six
  2. Run the app
  3. Notice the heading styling

Review

@[USER_NAME]

Make sure strings will be translated:

  • If there are new strings that have to be translated, I have added them to the client's strings.xml as a part of the integration PR.

@planarvoid planarvoid requested review from danilo04 and khaykov July 13, 2022 09:52
@planarvoid planarvoid marked this pull request as ready for review July 13, 2022 09:52
@khaykov khaykov self-assigned this Jul 14, 2022
Copy link
Member

@khaykov khaykov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't touched the Aztec for a while, but from what I can tell the size and color of Heading seems to work as expected. But it looks like something funky is happening with paddings? If you add a text after H1, and convert it to, say, H3, the paddings below H3 shift in weird way. This issue is present to some degree in trunk, with bigger headings, but behavior on the gif bellow is recent development:

Image from Gyazo

Comparison using same text sizes:
Image from Gyazo

Also, it looks like there are some changes around paragraph margin/padding, but I'm not sure what they affect here?

app/src/main/res/layout/activity_main.xml Outdated Show resolved Hide resolved
@planarvoid
Copy link
Contributor Author

you're right @khaykov . However, I've tested trunk and it's happening there as well so I don't think it's related to these changes.

I have no idea why the edit text isn't redrawn when the heading is added/removed. I've tried a few approaches and nothing redraws it so I've added a hacky fix which adds and removes a new line when a heading is applied/removed. It works well so let me know what you think!

@khaykov
Copy link
Member

khaykov commented Jul 17, 2022

@planarvoid Got it! While fix definitely helps avoid some crazy situations, I think it is a bit too hacky, and I can't really be sure it will not lead to some weird behavior down the line :) I think it would be a good idea to have a dedicated effort on this one, since it looks like other elements apart from heading are also affected by it, as described in #931.

@planarvoid
Copy link
Contributor Author

yeah, it just ignores the updated line height for all the elements. It's just much more prominent for the headings because the line height there is much bigger. I'll revert the change and we can leave it for a separate fix 👍

Copy link
Member

@khaykov khaykov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes, @planarvoid ! Looks good to me 👍

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