Feature/sup 2150 ext acc card add profile url link

[vikasrohit]

@parthshah @nlitwin @tladendo please suggest any change required to make the code robust and bug free. Specifically, do you guys see any other alternative to using preventStopPropagation directive, for avoiding opening of two links, when clicked on the URL inside a external web link card?

[parthshah]

why do we need to check both account.status and account.data.status? is the API not consistent?

[parthshah]

instead of using a div would it help to use anchor tag here? We wouldn't need an anchor tag for the url in that case..

[nlitwin]

I guess that depends on whether we want the whole thing clickable or just the anchor tag below. But we can delete div since we have an .external-link-tile class :)

[vikasrohit]

We are maintaining different nesting for added external account and web link. I was thinking to clean it up, however, it would have required more time and testing. Along with that, I was not sure which data structure to cleanup. For external accounts, we are nesting the added external account one more deep level e.g.

{
    status: 'SUCCESS' | 'ERROR',
    linkedAccount: {
        provider: 'github',
        data: {
            handle: '',
            status: 'PENDING',
            ...
        }
    }
}

while for external web links, it is

{
    provider: 'weblink',
    data: {
        handle: '',
        url: '',
        title: '',
        status: 'PENDING'
    }
}

Can you please suggest which one to fix?

[vikasrohit]

The rationale behind not using anchor tag is that it defeats the purpose of the anchor tag. I think it is not recommended to render a complicated HTML inside an anchor tag. Regarding the anchor tag on URL, we can still avoid it because if I remove the preventEventPropagation directive from it, it would bubble the event to the div and it would take user to the link. Only thing would be that we have to style the URL text as anchor.

[parthshah]

I think the requirement is to make the whole card clickable, hence my suggestion of replacing div w/ anchor tag :)

[vikasrohit]

@parthshah @nlitwin I have created issue 605 for tracking the data structure change and regarding replacing the div with anchor tag, I think we need more discussion because using anchor tag can cause slightly complicated logic for handling pending state cards and styling the URL field to look like an anchor.
I am going ahead to merge the PR into dev to get it QA'ed. If you guys like to have a separate issue to track the anchor thing, please let me know and if you guys agree with my rationale, we are good to go. :)