feat(about): add translatable logoAlt input to SiAboutComponent#2052
feat(about): add translatable logoAlt input to SiAboutComponent#2052chintankavathia wants to merge 9 commits into
Conversation
…kerConfig BREAKING CHANGE: `CriterionDefinition.datepickerConfig` type narrowed The type of `datepickerConfig` in `CriterionDefinition` (filtered-search) changed from `DatepickerInputConfig` to `Omit<DatepickerInputConfig, 'enableDateRange' | 'enableTwoMonthDateRange'>`. The properties `enableDateRange` and `enableTwoMonthDateRange ` are no longer accepted in `datepickerConfig` when used with filtered-search criteria, as they had no effect in that context. Migration: Remove any `enableDateRange` and `enableTwoMonthDateRange` properties from your `datepickerConfig` objects passed to `CriterionDefinition`.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a translatable logoAlt input to the SiAboutComponent, enabling localized alternative text for the application logo with support for appName interpolation. The implementation replaces hardcoded string concatenation in the template with the translate pipe and adds a default localized string. Accompanying changes include updates to the translatable keys interface, unit tests for the new input, and updated Playwright snapshots. I have no feedback to provide as there were no review comments to assess.
chintankavathia
force-pushed
the
fix/about/app-logo-alt-text
branch
from
1e75462 to
470657d
Compare
| * t(() => $localize`:@@SI_ABOUT.LOGO_ALT:{{appName}} Logo`) | ||
| * ``` | ||
| */ | ||
| readonly logoAlt = input<TranslatableString>( |
There was a problem hiding this comment.
Is the TranslatableString necessary?
| * ``` | ||
| */ | ||
| readonly logoAlt = input<TranslatableString>( | ||
| t(() => $localize`:@@SI_ABOUT.LOGO_ALT:{{appName}} Logo`) |
There was a problem hiding this comment.
According our writing guide I would assume 'logo' should be lowercase, or is uppercase required?
There was a problem hiding this comment.
This is inline with MS Outlook
There was a problem hiding this comment.
@kfenner @panch1739 Shall we write logo with capital 'L' or lowercase?
There was a problem hiding this comment.
@chintankavathia @spliffone I'm not sure, you can try to apply the writing style guide (maybe ask AI). But ultimately, this should be synched with the common texts from our language packs and how it's handled there.
/cc @michael-smt @Danisand
chintankavathia
force-pushed
the
fix/about/app-logo-alt-text
branch
from
470657d to
9e1a1ce
Compare
Replace hardcoded logo alt text with a configurable `logoAlt` input
that supports i18n via `TranslatableString` and `{{appName}}` interpolation.
chintankavathia
force-pushed
the
fix/about/app-logo-alt-text
branch
from
9e1a1ce to
645068b
Compare
4 participants
Replace hardcoded logo alt text with a configurable
logoAltinput that supports i18n viaTranslatableStringand{{appName}}interpolation.Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: