Connected Applications: Update to FoldableCard Pattern

[alternatekev]

As part of my attempt to replace custom foldable-card style components with the standard global FoldableCard component, this PR takes the Connected Applications screen, which looks like this and is built with a custom set of set of components and SCSS...

...and turns it into this, which uses the standard FoldableCard component:

This PR brings Connected Applications into parity with the new Sharing page, PR #1474.

To test: check out the branch and head on over to /me/security/connected-applications and you should see the updated layout.

[kellychoffman]

I'm not sure it should have the 'remove link' icon there. The "Disconnect" text on its own says it all.

[alternatekev]

@kellychoffman Do you think that's true of the screenshots in #1214 as well? I'd rather these two instances share the same treatment, although one is scary and one isn't.

[kellychoffman]

I missed the sharing PR. I'm not sure its necessary there. Why change it?

[alternatekev]

@kellychoffman The goal is just to keep them in sync because they say and more or less do the same thing. The difference in their "undo-ability" is indicated by the red color of the button in this PR. My goal is for anything that says "Disconnect" to work/look the same way. If I changed it here per your recommendation I'd be changing the Disconnect pattern.

Also, these two screens both use a wonky faux FoldableCard pattern that should also be synced up in a future PR, so I'd rather keep everything about these screens working/looking the same.

[kellychoffman]

My goal is for anything that says "Disconnect" to work/look the same way

I think there can be different degrees of severity. Disconnect that could break something and disconnect that will turn something off. Sharing is an example of the latter. You disconnect Facebook, and nothing breaks, it just prevents you from Publicizing to Facebook automatically in the future. I don't think this warrants a scary style. What I could see happening here, however is a confirmation dialog.

Also, these two screens both use a wonky faux FoldableCard pattern that should also be synced up in a future PR

👍 Totally agree.

[MichaelArestad]

If the icon isn't clarifying anything, we should drop it. In this case, I don't think it's accurate or particularly helpful. I also would suggest removing the scary styles as these are non-destructive changes like Kelly mentioned.

Can you add a screenshot showing both the connect and disconnect buttons at once? How well do they work together?

[alternatekev]

@MichaelArestad These are totally destructive changes. Revoking a connected application means you can't log in to that app with WordPress.com without re-signing in. The sharing connections are pretty non-destructive, though.

[alternatekev]

@kellychoffman @MichaelArestad They look like this now:

[kellychoffman]

👍

[alternatekev]

I've updated this PR to look more like #1474:

[kellychoffman]

The only issue I have is with the expand icon so close to the disconnect button. This will especially be an issue on smaller screens. I designed the original Sharing "cards" with this in mind, placing the open icon on the left.

[alternatekev]

@kellychoffman Why does the button being important mean it can't be next to expand/collapse? Would it be too easy to click the button on accident?

FoldableCard looks like a SectionHeader when opened, so I feel like we should be using the compact buttons there, fwiw.

Is something like this out of the question, do you think?

[kellychoffman]

Why does the button being important mean it can't be next to expand/collapse? Would it be too easy to click the button on accident?

Yep, thats all I meant.

I think the design above is quite nice. I like the visual separation between the button and the expand/collapse icons.

[folletto]

👍

[kellychoffman]

+1 to non scary disconnect buttons with a separator between it and the expand/collapse icon.

[dmsnell]

@alternatekev one observation (that kind of seems a bit late here) is that your screenshots looked like they were essentially making an accordion, of which we already have a component (albeit without the header buttons). Have we considered using the <Accordion />, updating the <Accordion />, or switching other old code from the <Accordion /> to get rid of it?

Are these altogether different use cases? (it appeared like we'd only want a single <FoldableCard /> open any given time…)

[alternatekev]

Have we considered using the , updating the , or switching other old code from the to get rid of it?

Accordion seems designed for use specifically in the Sidebar, and defaults to a single module open at a time. These screens never included a single-module limitation, and it doesn't seem like FoldableCard comes with one either. I still think FoldableCard is the right component here. Having more than one open at a time is fine on Sharing and Connected Applications.

[folletto]

What @alternatekev said. ;)

And:

I still think FoldableCard is the right component here. Having more than one open at a time is fine on Sharing and Connected Applications.

Having more than one open is most of the time the right thing, overall. ;)

[dmsnell]

cool. just making sure 😄

[alternatekev]

@dmsnell I've gone and made those updates :)

[dmsnell]

Since we are already rewriting the import statements, we could/should go ahead and rewrite the export too methinks

export default React.createClass( {
    …
} )

[dmsnell]

@alternatekev another few comments. Thanks for the updates. I absolutely ❤️ all the 🔻 removed lines of code in this PR 😄

[alternatekev]

@dmsnell done and done

[dmsnell]

Code looks good @alternatekev!

[kellychoffman]

Late to the party, but I 👍 this merge :)