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

Toggled flyout on edit thread click #2735

Closed
wants to merge 1 commit into from

Conversation

denichodev
Copy link
Contributor

Status

  • WIP
  • Ready for review
  • Needs testing

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

  • hyperion (frontend)

(Wasn't sure if hyperion needs to be redeployed.)

Release notes for users (delete if codebase-only change)

  • Minor bug fix where thread flyout not closing after editing a thread.

@denichodev
Copy link
Contributor Author

This resolves https://github.com/withspectrum/spectrum/issues/2634, might need some testing @mxstbr

Copy link
Contributor

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

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

Thank you so much for taking the time to contribute! Unfortunately, we already have a PR open for this at #2731—since they were first I'll close this one.

Thanks again, and I hope you'll be back for another PR!

@mxstbr mxstbr closed this Apr 5, 2018
@brianlovin
Copy link
Contributor

I think this PR was for a separate issue than #2731 -

That being said, I can't repro a call where the thread edit flyout doesn't close after editing a thread. @denichodev do you have a gif or repro of the bug?

@mxstbr
Copy link
Contributor

mxstbr commented Apr 5, 2018

Oh yeah, oopsie, totally my bad! 🙈 Reopening!

@mxstbr mxstbr reopened this Apr 5, 2018
@cutjavascript
Copy link
Contributor

What is needed here?
What is the next step?

Does it need testing for #2634 ?

@brianlovin
Copy link
Contributor

@cutjavascript - was hoping for repro steps from @denichodev; I can't seem to trigger the bug where the dropdown is still open after editing a thread

@denichodev
Copy link
Contributor Author

denichodev commented Apr 12, 2018

@brianlovin Sorry for the late reply, was busy.

Repro steps:

  1. Create a new thread
  2. Open the thread and click the gear icon for options like Edit, Lock and Delete (You have to actually click it)
  3. Click edit, then edit something and save them.
  4. Options are still open, different behavior from hovering the gear.

After doing some re-check, this PR actually reverse the behavior for hover and click. I'll check on it tomorrow 😄

@brianlovin
Copy link
Contributor

@denichodev saw you wanted to look into this more, but in my local testing it seems to fix the bug. Did you want to check anything else here or are you good to ship?

Only thing I could think of as a possible improvement here would be to just wrap this component in our src/components/outsideClickHandler component. Right now that component does this hacky thing:

{flyoutOpen && (
            <div
              style={{
                position: 'fixed',
                left: 0,
                right: 0,
                top: 0,
                bottom: 0,
                background: 'transparent',
                zIndex: 3002,
              }}
              data-cy={'thread-dropdown-close'}
              onClick={() =>
                setTimeout(() => {
                  this.toggleFlyout(false);
                })
              }
            />
          )}

This is obviously pretty ugly. Would you be interested in trying to just wrap the dropdown in the outsideClickHandler component to fix this behavior?

You can see an example implementation of this in src/views/communityMembers/components/editDropdown.js if that's helpful!

@brianlovin
Copy link
Contributor

<3 this, but I think the behavior has been superseded by #2937

@brianlovin brianlovin closed this Apr 27, 2018
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.

4 participants