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

Highlight Part of String #492

Merged
merged 2 commits into from
May 12, 2023
Merged

Highlight Part of String #492

merged 2 commits into from
May 12, 2023

Conversation

kugel3
Copy link
Contributor

@kugel3 kugel3 commented May 12, 2023

Description

There are already several places in the app where we want to do this:

  • Create strings where some parts come as parameters
  • Highlight only some of the parts in a certain color

An example would in a dApp request: "The dApp named [dAppName] wants to connect" where the dAppName is of course a parameter. This can easily be handled with our existing localization scheme:

"The dApp named %s wants to connect"

The problem comes when we want to e.g. highlight part of this string, for example the dApp name, in a different color. The way it was solved is to split this in three strings:

"The dApp named"
[dApp name]
"wants to connect"

color them in different colors, and then assemble the result. This works for English, but won't work in other languages with different word order.

The solution in this PR is to use the ability of markdown to mark parts of a text as bold or italic. The PR then adds functionality to convert e.g. italic text to a certain color. Markdown is supported in localized strings, and it's easy for translators to understand.

Concretely, if we wanted to highlight the words "wants to connect" above in pink, we would make this our localised string:

let markdown = "The dApp named %s *wants to connect*"

Then in the view code we simply write:

AttributedString(markdown: markdown, emphasizedColor: .pink)

Screenshot

image

PR submission checklist

  • I have tested account to account transfer flow and have confirmed that it works

@@ -7,4 +7,34 @@ extension AttributedString {
public init(_ string: some StringProtocol, foregroundColor: Color) {
self = with(AttributedString(string)) { $0.foregroundColor = foregroundColor }
}

public init(markdown: some StringProtocol, emphasizedColor: Color) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The term "emphasized" can mean both "italic" and "bold", better call it "italic" since that is what you are doing below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bold is called stronglyEmphasized in the AttributedString world though.

Copy link
Contributor Author

@kugel3 kugel3 May 12, 2023

Choose a reason for hiding this comment

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

But I think something like convertItalicsTo is clearer. emphasizedColor is ambiguous, it could also mean the opposite, that we want to emphasize parts of the string that have the provided color.

return result
}()

static let bold: AttributeContainer = {
Copy link
Contributor

Choose a reason for hiding this comment

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

bold is not used? should be removed? or should als obe used in markdown:emphasizedColor init above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not used in that helper currently, but we could use both, if we want to e.g. transform bold into one color, and italic into something else. Could also be used in general, those AttributeContainers are very strange and clunky on their own. But I can remove it for now.

Copy link
Contributor

@CyonAlexRDX CyonAlexRDX left a comment

Choose a reason for hiding this comment

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

LGTM

@kugel3 kugel3 merged commit 3430353 into main May 12, 2023
@kugel3 kugel3 deleted the helper/highlight-parts-of-strings branch May 12, 2023 12:31
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants