Skip to content
This repository was archived by the owner on Oct 11, 2022. It is now read-only.

Conversation

@arthurdenner
Copy link
Contributor

When the upsell is closed, the action related to it becomes available through a floating button.

Status

  • WIP
  • Ready for review
  • Needs testing

Deploy after merge (delete what needn't be deployed)

  • hyperion (frontend)

Related issues (delete if you don't know of any)
Closes #4531.

When closed, the action related to the upsell becomes available through a floating button
@spectrum-bot
Copy link

spectrum-bot bot commented Feb 21, 2019

Warnings
⚠️

These modified files do not have Flow enabled:

  • src/components/buttons/index.js

Generated by 🚫 dangerJS

@arthurdenner
Copy link
Contributor Author

Do I need to add // @flow on src/components/buttons/index.js to get rid of the warning above?

@mxstbr mxstbr requested a review from brianlovin February 21, 2019 08:57
@mxstbr
Copy link
Contributor

mxstbr commented Feb 21, 2019

@arthurdenner do not worry about it, you are solid! 👍 Going to defer to @brianlovin for a UI review here

<FloatingButton
loading={isLoading}
onClick={!currentUser ? this.login : this.toggleSubscription}
icon={!currentUser ? 'door-enter' : 'plus'}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add the door-enter icon to the Login or sign up button as well now.

@brianlovin
Copy link
Contributor

@arthurdenner pulled this locally and started trying to make things feel good, but the more I worked on this the more I realized how much we can simplify this. Most of the content in that upsell is repetitive, and we're getting too weird with having to dismiss it. My last commit simplifies everything to just a floating button - want to pull that and take a look?

@arthurdenner
Copy link
Contributor Author

Hey, @brianlovin, I'm glad you simplified the UI, it's way better now.

I'd just suggest to remove the 16px padding applied on smaller screens in the JoinChannelContainer component, keeping it at 8px looks better imo.

@brianlovin
Copy link
Contributor

Agreed, good catch! Let's ship this :)

@brianlovin brianlovin merged commit bd4010b into withspectrum:alpha Feb 21, 2019
@brianlovin
Copy link
Contributor

Thanks again for getting the momentum going here @arthurdenner :)

@arthurdenner arthurdenner deleted the feat/hide-thread-upsell branch February 21, 2019 23:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Signed out 'join community' upsell on thread views covers most of the content

3 participants