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

[ISSUE #3208] Removed ContextMenu usage #3260

Merged
merged 1 commit into from
Feb 9, 2018
Merged

[ISSUE #3208] Removed ContextMenu usage #3260

merged 1 commit into from
Feb 9, 2018

Conversation

jeluard
Copy link
Contributor

@jeluard jeluard commented Feb 8, 2018 •
edited by asemiankevich
Loading

fixes #3208
edit: also #3244

Summary:

Moved away from ContextMenu usage

Steps to test:

  • Open Status
  • Make general sanity check tests
  • Validate all ContextMenu have been migrated to popup lists (e.g. in group chat settings - user profile, when showing profile keys)

status: ready

@jeluard jeluard self-assigned this Feb 8, 2018
@status-github-bot status-github-bot bot added this to REVIEW in Pipeline for QA Feb 8, 2018
alwx
alwx approved these changes Feb 8, 2018
View reviewed changes
Copy link
Contributor

@alwx alwx left a comment

Choose a reason for hiding this comment

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

👍

@goranjovic
Copy link
Contributor

^^ The same codeclimate positive.

flexsurfer
flexsurfer reviewed Feb 8, 2018
View reviewed changes
src/status_im/ui/components/context_menu.cljs
@@ -1,55 +0,0 @@
(ns status-im.ui.components.context-menu
Copy link
Member

Choose a reason for hiding this comment

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

my heart is bleeding, why it's so hard to watch, how someone removes code you so diligently wrote

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't cry! Ask @goranjovic , you get use to it :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing code is my favorite thing

Copy link
Contributor

Choose a reason for hiding this comment

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

Haha, true. Hey, I like deleting code too, just not when it's barely one month old.

@status-github-bot status-github-bot bot moved this from REVIEW to CONTRIBUTOR in Pipeline for QA Feb 8, 2018
@goranjovic
Copy link
Contributor

@jeluard Please resolve the conflicts

@jeluard jeluard force-pushed the feature/#3208 branch 2 times, most recently from df96a3f to c43e53c Compare February 8, 2018 14:48
@jeluard
Copy link
Contributor Author

jeluard commented Feb 8, 2018

@goranjovic Done

@goranjovic goranjovic moved this from CONTRIBUTOR to TO TEST in Pipeline for QA Feb 8, 2018
@status-github-bot status-github-bot bot moved this from TO TEST to CONTRIBUTOR in Pipeline for QA Feb 8, 2018
@jeluard jeluard moved this from CONTRIBUTOR to TO TEST in Pipeline for QA Feb 8, 2018
@asemiankevich
Copy link
Contributor

asemiankevich commented Feb 9, 2018 •
edited
Loading

Branch: PR-3260
Android: https://i.diawi.com/GzgafF
iOS: https://i.diawi.com/qzUR6q

java.lang.RuntimeException: Error calling RCTDeviceEventEmitter.emit on android when starting the build.. iOS build does not start at all

@asemiankevich asemiankevich moved this from TO TEST to IN TESTING in Pipeline for QA Feb 9, 2018
@status-im status-im deleted a comment from statustestbot Feb 9, 2018
@asemiankevich
Copy link
Contributor

ARTIFACT iOS: https://i.diawi.com/zKiaEN
ARTIFACT Android: https://i.diawi.com/AS8ygH

@asemiankevich asemiankevich moved this from IN TESTING to MERGE in Pipeline for QA Feb 9, 2018
@jeluard @flexsurfer
[ISSUE #3208] Removed ContextMenu usage
1e262fc 
Signed-off-by: Andrey Shovkoplyas <motor4ik@gmail.com>
@flexsurfer flexsurfer merged commit 1e262fc into develop Feb 9, 2018
Pipeline for QA automation moved this from MERGE to DONE Feb 9, 2018
@asemiankevich asemiankevich moved this from DONE to Cherry pick for release in Pipeline for QA Feb 9, 2018
@asemiankevich asemiankevich mentioned this pull request Feb 12, 2018
Closed
@goranjovic goranjovic mentioned this pull request Feb 12, 2018
Merged
@yenda yenda moved this from Cherry pick for release to RELEASE 0.9.14 in Pipeline for QA Feb 13, 2018
@rasom rasom deleted the feature/#3208 branch April 19, 2018 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goranjovic goranjovic goranjovic approved these changes

@alwx alwx alwx approved these changes

@yenda yenda yenda approved these changes

@flexsurfer flexsurfer flexsurfer approved these changes

Assignees

@jeluard jeluard

Labels
Tested - OK
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate off MenuContext
6 participants
@jeluard @goranjovic @asemiankevich @alwx @yenda @flexsurfer