848778 - [SMS] Create a 'generic' option menu for new features (i.e. bug 838000)

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Borja Salguero [:borjasalguero]]

Due to new features in MMS/SMS App need a generic way of creating an 'action-menu', we need a generic way of generating this menu on the fly. This could be part of '/shared' folder in the future.

Comment 1

[Borja Salguero [:borjasalguero]]

Attachment: Pull Request

Comment 2

[Borja Salguero [:borjasalguero]]

The goal of this patch is creating a generic way of creating the following type of screen https://bug838000.bugzilla.mozilla.org/attachment.cgi?id=711610

Comment 3

[Dietrich Ayala (:dietrich)]

This should block 1.1 then yes?

Comment 4

[Borja Salguero [:borjasalguero]]

Yep, due to it's needed for other bugs which are blocking 1.1 as well

Comment 5

[satender]

Comment on attachment 722321 [details]
Pull Request

the changes to action_menu.js look good. one final comment in pull request is to move the file to shared/ folder. marking it as r+ from side.

Comment 6

[Rick Waldron [:rwaldron]]

r+

Comment 7

[Borja Salguero [:borjasalguero]]

Thanks Rick & Satender for your review!Gracias! ;)

Comment 8

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted commit 34ec6dd4092f8f6455ae7296f1d1a1a445c1164d as:
v1-train: 5a59e1ae55c8179adfa8ab85e061f57efdaf1186

Comment 9

[Kan-Ru Chen [:kanru] (UTC+9)]

This broke install-gaia on v1-train because responsive.css was not checked in together.

  build/webapp-zip.js:311: Error: Error: Can't add inexistent file to zip : /home/kanru/zone2/mozilla/B2G/gaia/shared/style/responsive.css from: sms.gaiamobile.org

Comment 10

[[:fabrice] Fabrice Desré]

backed out from v1-train: https://github.com/mozilla-b2g/gaia/commit/4073f456f83afb39b8d9d56d462726e3daa94859

Comment 11

[Andreas Gal :gal]

Guys, please remember to run tests before committing.

Comment 12

[Julien Wajsberg [:julienw]]

My understanding was that we don't want new SMS features to land on v1-train for now, we want everything to land together. That was the cause of this problem.

These bugs should be marked with [NO_UPLIFT] in title.

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to Julien Wajsberg [:julienw] from comment #12)
> My understanding was that we don't want new SMS features to land on v1-train
> for now, we want everything to land together. That was the cause of this
> problem.
> 
> These bugs should be marked with [NO_UPLIFT] in title.

This bug is blocking bug 850582, which does not have NO_UPLIFT.  Should we still hold off on this uplift?

Comment 14

[Julien Wajsberg [:julienw]]

go for it; I was told that SMS-only features can land.

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 34ec6dd4092f8f6455ae7296f1d1a1a445c1164d
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 16

[Julien Wajsberg [:julienw]]

taking uplift

Comment 17

[Julien Wajsberg [:julienw]]

v1-train: cdfc1e6472016c5015fce2dcd8a5dc07b97c96cc

Comment 18

[Borja Salguero [:borjasalguero]]

Comment on attachment 722321 [details]
Pull Request

This was landed weeks ago.