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.

Sign up for GitHub

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

Jump to bottom

Local identities #656

Merged
merged 107 commits into from Mar 29, 2019
Merged

Local identities #656

merged 107 commits into from Mar 29, 2019

Conversation

AquiGorka
Copy link
Contributor

@AquiGorka AquiGorka commented Mar 20, 2019 •
edited

This PR aggregates changes from:

Accompanying apps PRs:

Todo

  • Bump minor version of @aragon/ui once released with updated IdentityBadge
  • Bump version of @aragon/wrapper once released with identities functionality

@AquiGorka AquiGorka force-pushed the feature/local-identities branch 2 times, most recently from 211cb3a to 23a5593 Compare March 20, 2019 09:00
@AquiGorka AquiGorka mentioned this pull request Mar 20, 2019
Merged
2 tasks
@2color 2color mentioned this pull request Mar 21, 2019
Merged
2 tasks
2color
2color reviewed Mar 22, 2019
View reviewed changes
src/App.js Show resolved Hide resolved
2color
2color reviewed Mar 22, 2019
View reviewed changes
src/App.js Show resolved Hide resolved
2color
2color reviewed Mar 22, 2019
View reviewed changes
src/App.js Outdated Show resolved Hide resolved
2color
2color reviewed Mar 22, 2019
View reviewed changes
src/App.js Outdated Show resolved Hide resolved
@2color
Copy link
Contributor

2color commented Mar 22, 2019

I'm getting the following error when interacting with identities in the native setting apps

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
in Modal (created by Spring)
in div (created by AnimatedComponent)
in AnimatedComponent (created by ForwardRef)
in ForwardRef (created by Context.Consumer)
in StyledComponent (created by ModalManager__AnimatedWrap)
in ModalManager__AnimatedWrap (created by Spring)
in div (created by Context.Consumer)
in StyledComponent (created by ModalManager__Wrap)
in ModalManager__Wrap (created by Spring)
in Spring (created by KeyframesImpl)
in KeyframesImpl (created by ForwardRef)
in ForwardRef (created by Transition)

Steps to recreate:

  1. Click on a badge
  2. Edit label
  3. Save
  4. Repeat first three steps
  5. See error after the second save

Leaving this comment so I can debug later.

@AquiGorka
Copy link
Contributor Author

Memory leaked fixed in c6741b2

@2color 2color self-requested a review March 25, 2019 15:28
2color
2color approved these changes Mar 25, 2019
View reviewed changes
Copy link
Contributor

@2color 2color left a comment

Choose a reason for hiding this comment

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

Looking great!

bpierre
bpierre approved these changes Mar 25, 2019
View reviewed changes
src/components/MenuPanel/MenuPanel.js Outdated Show resolved Hide resolved
src/components/Preferences/Preferences.js Outdated Show resolved Hide resolved
src/components/Preferences/Preferences.js Show resolved Hide resolved
src/components/Preferences/Preferences.js Outdated Show resolved Hide resolved
src/components/Preferences/Preferences.js Outdated Show resolved Hide resolved
src/components/LocalIdentityModal/LocalIdentityModal.js Outdated Show resolved Hide resolved
src/components/LocalIdentityModal/LocalIdentityModal.js Outdated Show resolved Hide resolved
src/components/LocalIdentityModal/LocalIdentityModal.js Outdated Show resolved Hide resolved
src/components/LocalIdentityModal/LocalIdentityModal.js Show resolved Hide resolved
src/components/App/AppIFrame.js Show resolved Hide resolved
@bpierre bpierre mentioned this pull request Mar 25, 2019
Merged
2 tasks
This was referenced Mar 26, 2019
Open
Open
Closed
Closed
Closed
Closed
Closed
@2color 2color mentioned this pull request Mar 28, 2019
Merged
2color and others added 15 commits March 29, 2019 09:27
@2color @AquiGorka
Add exit on escape functionality
c3218ee
@2color @AquiGorka
Upgrade aragon/wrapper to version with local identities
16e5c8c
@AquiGorka
MenuPanel: add small size to preferences button
a88282e
@AquiGorka
Preferences: update title
2f71d80
@AquiGorka
Update keyup listeners to keydown
d3a1a94
@AquiGorka
Rename modify hook to event to avoid confusion with react hooks
92774ed
@AquiGorka
EmptyLocalIdentities: fix badge wrap
1da26f1
@izqui @AquiGorka
LocalIdentities: update file name
cfb6b27 
Co-Authored-By: AquiGorka <gorka@aquigorka.com>
@AquiGorka
Preferences: remove coming soon component for time being
5669b77
@AquiGorka
LocalIdentities: update export file name
ee17b80
@AquiGorka
MenuPanel: update preferences button alignments
494eb26
@AquiGorka
Import: fix import for mobile safari
10136b6
@AquiGorka
MenuPanel: add scroll if needed
725aeca
@AquiGorka
LocalIdentities: remove export button for ios devices
f873bad
@AquiGorka
LocalIdentityModal: update format for ci's linter
69c6b71
@AquiGorka AquiGorka merged commit 77e1225 into master Mar 29, 2019
@AquiGorka AquiGorka deleted the feature/local-identities branch March 29, 2019 09:47
@AquiGorka AquiGorka mentioned this pull request Mar 29, 2019
Closed
3 tasks
@2color 2color mentioned this pull request Apr 3, 2019
Closed
2color added a commit that referenced this pull request Apr 3, 2019
@2color
Merge remote-tracking branch 'origin/master' into activity-panel
2e5cde9 
* origin/master: (56 commits)
  Identity - Improve LocalIdentityBadge (#673)
  Menu panel footer separator (#666)
  fix(MenuPanel): avoid clickable margin above system apps toggle (#671)
  Local identities (#656)
  MenuPanel: add toggle animation to show/hide system apps (#658)
  Add github workflow for linting/building (#663)
  Permissions: added system and background app labels (#650)
  Use the same component to render every app icon (#655)
  chore: add all contributors and contributing guidelines (#649)
  Manage the menu button using messages + prevent re-mounting on resize (#651)
  Update melon (#647)
  Apps <> System apps separator (#648)
  Upgrade lint-staged (#646)
  fix: always leave Kernel as first app (#645)
  fix: avoid assigning a registry tag if app is not on a registry (#644)
  chore: upgrade @aragon/wrapper to v4.0.0-beta.1 (#639)
  DaoSettings: add bottom margin on app items (#638)
  Refactor common components (#615)
  Enforce MenuPanel’s width (#636)
  Menu panel swipe open close (#606)
  ...
2color added a commit that referenced this pull request Apr 3, 2019
@2color
Merge remote-tracking branch 'origin/master' into notifications
44a19c4 
* origin/master: (55 commits)
  Identity - Improve LocalIdentityBadge (#673)
  Menu panel footer separator (#666)
  fix(MenuPanel): avoid clickable margin above system apps toggle (#671)
  Local identities (#656)
  MenuPanel: add toggle animation to show/hide system apps (#658)
  Add github workflow for linting/building (#663)
  Permissions: added system and background app labels (#650)
  Use the same component to render every app icon (#655)
  chore: add all contributors and contributing guidelines (#649)
  Manage the menu button using messages + prevent re-mounting on resize (#651)
  Update melon (#647)
  Apps <> System apps separator (#648)
  Upgrade lint-staged (#646)
  fix: always leave Kernel as first app (#645)
  fix: avoid assigning a registry tag if app is not on a registry (#644)
  chore: upgrade @aragon/wrapper to v4.0.0-beta.1 (#639)
  DaoSettings: add bottom margin on app items (#638)
  Refactor common components (#615)
  Enforce MenuPanel’s width (#636)
  Menu panel swipe open close (#606)
  ...
sohkai
sohkai reviewed Apr 4, 2019
View reviewed changes
src/components/Preferences/EmptyLocalIdentities.js
left: 2px;
`}
/>
<IdentityBadge entity="Address badge" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like we should add a way to stop the popover from activating @bpierre

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bpierre bpierre bpierre approved these changes

@izqui izqui izqui left review comments

@luisivan luisivan luisivan left review comments

@2color 2color 2color approved these changes

@sohkai sohkai sohkai left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@AquiGorka @2color @bpierre @izqui @luisivan @sohkai