Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reliable merge box handlers #2019

Merged
merged 9 commits into from May 17, 2019

Conversation

Projects
None yet
2 participants
@bfred-it
Copy link
Collaborator

commented May 9, 2019

Depends on sindresorhus/mem#35

Maybe these could benefit from this as well, once they're merged:

  • #1950 (edit: no, the button appears outside the form, not inside)
  • #2023 ( 966e14b)

Reason of the changes:

  • details:toggled was unreliable (small fix)
  • fit-textareas was being called from an unrelated feature
  • add-co-authored-by wasn't being added when the session was restored (i.e. type a new PR title and reload the page)
  • add-co-authored-by was listening to a click on a button, which conflicted with the future #1950

bfred-it added some commits May 9, 2019

Move `fit-textarea` callback out of `sync-pr-commit-title`
And make it more reliable. Firefox and Chrome had opposite `event.detail.open` values
Make `add-co-authored-by` more reliable
Now also appears after `session:resume` and doesn't add unnecessary line-breaks

@bfred-it bfred-it referenced this pull request May 9, 2019

Merged

Add `update-pr-from-base-branch` feature #1950

7 of 7 tasks complete
callback(event);
}
});

This comment has been minimized.

Copy link
@bfred-it

bfred-it May 10, 2019

Author Collaborator

I love this pattern.

Since addEventListener doesn't add the same handler twice, it's important that the same exact handler is passed every time. To do this I always use a WeakMap and an ugly sequence of get or create, then set and return, but I think I finally found a way to hide this complexity with mem.

This comment has been minimized.

Copy link
@sindresorhus

sindresorhus May 17, 2019

Owner

I like this pattern a lot 👍

This comment has been minimized.

Copy link
@bfred-it

bfred-it May 17, 2019

Author Collaborator

Also used here bfred-it/delegate-it#10

Screenshot 2019-05-17 at 21 58 06

@bfred-it bfred-it marked this pull request as ready for review May 15, 2019

@bfred-it
Copy link
Collaborator Author

left a comment

This PR is good to go too

@bfred-it bfred-it requested a review from sindresorhus May 15, 2019

@bfred-it bfred-it merged commit 1e631d2 into master May 17, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@bfred-it bfred-it deleted the reliable-merge-box-handlers branch May 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.