EPMEDU-4856/Fix invalid email in FAQ#287
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes an invalid email address in the FAQ section and styles it as a clickable link with blue color and underline. The change updates the implementation from using markdown-style links with LocalizedStringKey to using AttributedString with explicit link and styling attributes.
Changes:
- Replaced markdown-style email link with
AttributedStringimplementation that sets link, color, and underline attributes - Changed the
footerTextproperty type fromLocalizedStringKeytoAttributedStringin the view model and contract - Updated the view to use
.tint(.blue)for link styling instead of.accentColor()
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| FAQViewModel.swift | Implements AttributedString with explicit email link attributes including URL, foreground color, and underline style |
| FAQContract.swift | Updates footerText property type from LocalizedStringKey to AttributedString |
| FAQView.swift | Removes foregroundColor and accentColor modifiers, adds .tint(.blue) for link color |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .accentColor(designEngine.colors.textPrimary.color) | ||
| .multilineTextAlignment(.center) | ||
| .frame(maxWidth: .infinity) | ||
| .tint(.blue) |
There was a problem hiding this comment.
The hardcoded .blue color breaks the design system convention used throughout the codebase. All other views use designEngine.colors for color styling (e.g., designEngine.colors.textPrimary.color, designEngine.colors.textSecondary.color). This should use an appropriate color from the design engine instead. Consider adding a link or accent color to the design engine if one doesn't exist, or use an existing color that matches the design requirements.
There was a problem hiding this comment.
please check suggest to stick to design engine based project colors if not any special circumstances
|
|
||
| if let range = text.range(of: email) { | ||
| text[range].link = URL(string: "mailto:\(email)") | ||
| text[range].foregroundColor = .blue |
There was a problem hiding this comment.
The hardcoded .blue color for the email link breaks the design system convention. The codebase consistently uses designEngine.colors for all color styling. If a link-specific color is needed, it should be added to the design engine's color palette rather than hardcoded here.
| Text(viewModel.footerText) | ||
| .textSelection(.enabled) | ||
| .font(designEngine.fonts.primary.regular(16)?.font) | ||
| .foregroundColor(designEngine.colors.textPrimary.color) | ||
| .accentColor(designEngine.colors.textPrimary.color) | ||
| .multilineTextAlignment(.center) |
There was a problem hiding this comment.
The removal of .foregroundColor(designEngine.colors.textPrimary.color) means the non-link text in the footer will no longer have a defined color and will use the system default. This is inconsistent with other text elements in the view (like headerText at line 47 and text in QuestionRow at lines 101 and 114) which explicitly set foregroundColor using the design engine. The footer text should maintain consistent styling with the rest of the view.
| .accentColor(designEngine.colors.textPrimary.color) | ||
| .multilineTextAlignment(.center) | ||
| .frame(maxWidth: .infinity) | ||
| .tint(.blue) |
There was a problem hiding this comment.
No need all these changes. .tint is enough.
And it is better to use designEngine instead of hardcoded values.
.tint(designEngine.colors.elementSpecial.color)
What was done
-Fixed the invalid email address and styled it as a clickable link with blue color and underline
Related Jira task
EPMEDU-4856
