Closed
Bug 1155542
Opened 10 years ago
Closed 10 years ago
[Messages][New Gaia Architecture] Centralizing the global components/styling into a folder
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: steveck, Assigned: steveck)
References
Details
(Whiteboard: [sms-sprint-2.2S11])
Attachments
(1 file)
Since view separations will also reorganize HTML/CSS/JS into different folder, we'll also need to move global JS like utils.js or root.css into a "shared" folder in message.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → schung
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8593885 [details]
[gaia] steveck-chung:new-message-global > mozilla-b2g:master
Hi guys,
It's simply a rough idea about moving the "global"(because "shared" is for gaia shared component currently and I don't want them mixed up, but maybe you have better naming for this) assets(JS/CSS/others) into a subfolder since we might create subfolders for views in the future. I also move the desktoptop testing out of the js folder because it seems weird to keep it inside(Although we will remove this in the build time). Not sure if this structure is suitable for current master or should be better for new architecture(or you think we should keep this structure even in new gaia structure). It's just a WIP and path is not handled correctly, and any suggestion is welcome.
Attachment #8593885 -
Flags: feedback?(felash)
Attachment #8593885 -
Flags: feedback?(azaaza)
Assignee | ||
Updated•10 years ago
|
Attachment #8593885 -
Flags: feedback?(azaaza) → feedback?(azasypkin)
Comment 3•10 years ago
|
||
Comment on attachment 8593885 [details]
[gaia] steveck-chung:new-message-global > mozilla-b2g:master
Looks good to me, left some thoughts at GitHub. Generally speaking I'd rather move forward with smth like this sooner than later and we can refine this along the way to new structure.
Attachment #8593885 -
Flags: feedback?(azasypkin) → feedback+
Comment 4•10 years ago
|
||
Comment on attachment 8593885 [details]
[gaia] steveck-chung:new-message-global > mozilla-b2g:master
feedback given on github
Attachment #8593885 -
Flags: feedback?(felash)
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8593885 [details]
[gaia] steveck-chung:new-message-global > mozilla-b2g:master
Hi all,
The patch updated based on Oleg's new structure proposal, with the desktop-mock/app icon path changes. I tried to move the setup.js to shared but in vain :/ maybe I'm wrong bug I can't find a correct way to change setup.js load path.
Attachment #8593885 -
Flags: review?(felash)
Attachment #8593885 -
Flags: review?(azasypkin)
Comment 6•10 years ago
|
||
(In reply to Steve Chung [:steveck] from comment #5)
> Comment on attachment 8593885 [details]
> [gaia] steveck-chung:new-message-global > mozilla-b2g:master
>
> Hi all,
>
> The patch updated based on Oleg's new structure proposal, with the
> desktop-mock/app icon path changes. I tried to move the setup.js to shared
> but in vain :/ maybe I'm wrong bug I can't find a correct way to change
> setup.js load path.
Yeah the path is hardcoded in [1].
[1] https://github.com/mozilla-b2g/gaia/blob/392ef2fafa48166378fb18e33da1ab4e04235e66/dev_apps/test-agent/common/test/agent_proxy.js#L26
Assignee | ||
Updated•10 years ago
|
Whiteboard: [sms-sprint-2.2S11]
Comment 7•10 years ago
|
||
Comment on attachment 8593885 [details]
[gaia] steveck-chung:new-message-global > mozilla-b2g:master
I think we'll see better after bug 1156631.
Also I don't think we should move everything but only views-related files. For example time_headers.js should move, but threads.js shouldn't. Tell me what you think.
Attachment #8593885 -
Flags: review?(felash)
Comment 8•10 years ago
|
||
Comment on attachment 8593885 [details]
[gaia] steveck-chung:new-message-global > mozilla-b2g:master
Yeah, let's wait for bug 1156631 first maybe.
> Also I don't think we should move everything but only views-related files.
> For example time_headers.js should move, but threads.js shouldn't.
I asked on Github, but if you mean that some of the files in sms/js/ will go to sms/services or sms/services/shared, then I yeah, I'd agree that we don't need to move them to views/shared/js. As for shared CSS and images - views/shared is the only good place IMO.
Attachment #8593885 -
Flags: review?(azasypkin)
Assignee | ||
Comment 9•10 years ago
|
||
I agree that we could set the backend/service-wise js into another folder(originally I didn't split these js files because they're not under the views, but now it's reasonable that some js shouldn't live in frontend side). I think maybe sms/services is enough and additional shared seem a little bit redundant. And about the utils.js, maybe I'll put it under views because most of functions are ui related, and split into ui/non-ui utils in the future?
Comment 10•10 years ago
|
||
Yeah, I agree with everyting you (Steve and Oleg) said here :)
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8593885 [details]
[gaia] steveck-chung:new-message-global > mozilla-b2g:master
Another update after css separation finished. I only move threads/message_manager/drafts into services because that's what we confirmed firstly, but I'm not sure the necessasity of other js. Maybe we can decide here or in the bug for bridge library.
Attachment #8593885 -
Flags: review?(felash)
Attachment #8593885 -
Flags: review?(azasypkin)
Comment 12•10 years ago
|
||
Comment on attachment 8593885 [details]
[gaia] steveck-chung:new-message-global > mozilla-b2g:master
Looks good! Just few leftovers in unit tests and desktop mock (commented at GitHub).
I think we'll move the rest of files more precisely in the next patches.
Thanks!
Attachment #8593885 -
Flags: review?(azasypkin) → review+
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Oleg Zasypkin [:azasypkin] from comment #12)
> Comment on attachment 8593885 [details]
> [gaia] steveck-chung:new-message-global > mozilla-b2g:master
>
> Looks good! Just few leftovers in unit tests and desktop mock (commented at
> GitHub).
>
> I think we'll move the rest of files more precisely in the next patches.
>
> Thanks!
Thanks for the catch! I found a weird issue about the XMLHttpRequest in unit test. xhr will return success with html blob even the given file is not existed. Since we don't check the actual file type(I don't think this is necessary anyway) our unit test will not be blocked by any error if assetHelper return a valid blob. I'll file a bug for it.
Comment 14•10 years ago
|
||
Comment on attachment 8593885 [details]
[gaia] steveck-chung:new-message-global > mozilla-b2g:master
I only have a small nit but otherwise this looks good, let's land this.
Attachment #8593885 -
Flags: review?(felash) → review+
Updated•10 years ago
|
Keywords: checkin-needed
Comment 16•10 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/f78ac92796e3aaf3000382886d7ce9be63aae4e7
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•