← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] ~ines-almeida/launchpad:social-accounts-ui-display-accounts into launchpad:master

 


Diff comments:

> diff --git a/lib/lp/app/browser/tales.py b/lib/lp/app/browser/tales.py
> index eb5cad2..862c0bb 100644
> --- a/lib/lp/app/browser/tales.py
> +++ b/lib/lp/app/browser/tales.py
> @@ -3049,3 +3050,27 @@ 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_displayname": "formatted_displayname",
> +    }
> +
> +    def formatted_displayname(self, view_name=None):
> +        if self._context.platform == SocialPlatform.MATRIX:
> +            nickname = self._context.identity.get("nickname")
> +            network = self._context.identity.get("network")

Does it make sense here to check if nickname and network are != None ?

> +            return structured(
> +                dedent(
> +                    f"""\
> +                    <strong>{nickname}</strong>
> +                    <span class="lesser"> on </span>
> +                    <strong>{network}</strong>
> +                """
> +                ),
> +            ).escapedtext
> +        return None
> diff --git a/lib/lp/registry/templates/person-portlet-contact-details.pt b/lib/lp/registry/templates/person-portlet-contact-details.pt
> index 9a45fd4..720415d 100644
> --- a/lib/lp/registry/templates/person-portlet-contact-details.pt
> +++ b/lib/lp/registry/templates/person-portlet-contact-details.pt
> @@ -200,6 +172,46 @@
>      </dl>
>    </div>
>  
> +  <div class="yui-u two-column-list">
> +  <h3>Social Accounts</h3>

I tested the UI locally and `Social Accounts` headers seems to get lost a bit among other headers and titles, is there a way to make it more prominent?

> +    <dl tal:condition="view/should_show_ircnicknames_section">
> +      <dt>IRC:
> +    <a tal:replace="structure overview_menu/editircnicknames/fmt:icon" />
> +    </dt>
> +      <dd tal:repeat="ircnick context/ircnicknames">
> +        <span tal:replace="structure ircnick/fmt:formatted_displayname"/>
> +      </dd>
> +      <dd tal:condition="not: context/ircnicknames">
> +        No IRC nicknames registered.
> +      </dd>
> +    </dl>
> +
> +    <dl tal:condition="view/should_show_jabberids_section" id="jabber-ids">
> +      <dt>Jabber:
> +        <a tal:replace="structure overview_menu/editjabberids/fmt:icon" />
> +      </dt>
> +      <dd>
> +        <tal:block repeat="jabberid context/jabberids">
> +          <span tal:replace="jabberid/jabberid/fmt:obfuscate-email"
> +            /><span tal:condition="not: repeat/jabberid/end">,</span>
> +        </tal:block>
> +        <div tal:condition="context/jabberids/is_empty">
> +          No Jabber IDs registered.
> +        </div>
> +      </dd>
> +    </dl>
> +
> +    <dl tal:condition="view/should_show_matrix_accounts_section" id="matrix-accounts">
> +      <dt>Matrix:</dt>
> +      <dd tal:repeat="matrix_account view/matrix_accounts">
> +        <span tal:replace="structure matrix_account/fmt:formatted_displayname"/>
> +      </dd>
> +      <dd tal:condition="not: view/matrix_accounts">
> +        No Matrix accounts registered.
> +      </dd>
> +    </dl>
> +  </div>
> +
>  </div>
>  
>  </tal:root>


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



Follow ups