launchpad-reviewers team mailing list archive

[Guruprasad <mp+458730@xxxxxxxxxxxxxxxxxx>]

Left a small comment. Don't forget to add appropriate tests.

Diff comments:

> diff --git a/lib/lp/app/browser/tales.py b/lib/lp/app/browser/tales.py
> index eb5cad2..9ec03a3 100644
> --- a/lib/lp/app/browser/tales.py
> +++ b/lib/lp/app/browser/tales.py
> @@ -3049,3 +3050,32 @@ class IRCNicknameFormatterAPI(ObjectFormatterAPI):
>              self._context.nickname,
>              self._context.network,
>          ).escapedtext
> +
> +
> +@implementer(ITraversable)
> +class SocialAccountFormatterAPI(ObjectFormatterAPI):
> +    """Adapter from social account objects to a formatted string."""
> +
> +    traversable_names = {
> +        "formatted_display": "formatted_display",
> +    }
> +
> +    def getPlatformClass(self):
> +        return SOCIAL_PLATFORM_TYPES_MAP.get(self._context.platform)
> +
> +    def icon(self, platform):
> +        return (
> +            f'<img class="social_accounts__icon" alt="{platform.title}" '
> +            f'title="{platform.title}" src="/@@/{platform.icon}" />'
> +        )
> +
> +    def formatted_display(self, view_name=None):

This formatting looks great but, imho, it is better to specify the platform name somehow because the icons are not standard and universally known.

> +        platform = self.getPlatformClass()
> +        icon = self.icon(platform)
> +        text_display = platform.display_format.format(**self._context.identity)
> +
> +        if platform.url:
> +            url = platform.url.format(**self._context.identity)
> +            text_display = f'<a href={url} target="_blank">{text_display}</a>'
> +
> +        return structured(f"{icon} {text_display}").escapedtext


-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/458730
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:social-accounts-display-view into launchpad:master.