[E-mail] Convert switches.css building blocks to web components

ASSIGNED
Assigned to

Status

Firefox OS
Gaia::E-Mail
ASSIGNED
2 years ago
2 years ago

People

(Reporter: kgrandon, Assigned: kgrandon)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
This should track porting current usage of pack-switch and pack-checkbox over to gaia-switch and gaia-checkbox.

Comment 1

2 years ago
Created attachment 8635839 [details] [review]
[gaia] KevinGrandon:bug_1185387_email_switch_components > mozilla-b2g:master
(Assignee)

Comment 2

2 years ago
(In reply to Autolander from comment #1)
> Created attachment 8635839 [details] [review]
> [gaia] KevinGrandon:bug_1185387_email_switch_components > mozilla-b2g:master

Basic implementation done, but shared resource import isn't working. Need to wrap my head around the e-mail build system a bit more to understand this.
(Assignee)

Comment 3

2 years ago
Hi James - I am looking for some help in the 2.5 cycle for either someone to drive web component implementation, or just provide some assistance/reviews. If you'd be interested in taking over this patch or driving it, great! Or if you just have time to just give me some pointers on where to look and reviews, also great!

The current strategy is to implement the existing components in the shared/ folder, which we've been doing with success across the majority of the apps. For e-mail, I'm running into some path loading problems, and I was thinking that you might be able to spot what's wrong much quicker than I could. An initial patch is attached to this bug, and I'm seeing errors like:

E/E-Mail  ( 9042): [JavaScript Error: "Error: Load failed: gaia_switch: app://email.gaiamobile.org/js/gaia_switch.js" {file: "app://email.gaiamobile.org/js/config.js" line: 705}]
E/E-Mail  ( 9042): [JavaScript Error: "Error: Load failed: gaia_checkbox: app://email.gaiamobile.org/js/gaia_checkbox.js" {file: "app://email.gaiamobile.org/js/config.js" line: 705}]
Flags: needinfo?(jrburke)
Created attachment 8636357 [details] [diff] [review]
bug_1185387_additional.patch

Attached is a patch that addresses the basic loading issues. The email app auto-discovers custom elements used in a card and will load them based on their tag name. There is a function that translates the tag name to module ID and that is done in the config.js file.

There are still some issues to sort out, but that is the main thing.

I am concerned about how shared/js/component_utils is included: it is an explicit dependency of the switch and checkbox, but it is not mentioned as one. I liked the idea of defining the custom elements as modules, with their dependencies listed, like gaia-header. As it is now, the components_utils happens to load first, but the implicit dependency makes me nervous.

I also wish there was a way to optimize the style scoped @import usage for the stylesheets. As it is now, they cannot be optimized by including it with other style sheets loaded on startup, and so I am concerned longer term around our startup pages using many custom elements in this style.

As it is now, the checkbox is used on the startup card message_list, but initially hidden, so likely not to cause an issue, but does not seem to scale generally.

Since the scoped style stuff is under flux, I suggest using CSS classes to target the elements, and even though that CSS would be global, not scoped, that pattern is robust to optimization and not dependent on the more unsettled parts of the custom elements.

Work list of issues to sort out if this goes ahead, so that I remember them later:

* Tapping on "Set as default email" when already checked should not result in an uncheck: once it is set, the user needs to go to the other account that they want to be the default and tap that unchecked checkbox.
* I think the switch values are reversed to what the existing code is expecting. Turning off a switch indicating setting the sound to "on" for "Play sound after message is sent".
* There are checkboxes in the message_list for bulk selection. Those need to be converted, and may need a message_list.js change too. Check search also, and mke sure startup cache still behaves as expected.
* Confirm CSS positioning/spacing of the new elements in the card, make sure they match placing before the patch.
* Check RTL.

I am unlikely to pick up this ticket soon: we are trying to get to a landable place for the email conversations work, and that branch has some other UI construction changes, so I prefer to land that conversations work first. 

I also feel like the custom elements stuff is still in flux, and given the lack of a clear UX refresh for 2.5 also not seeing an immediate need for the change.
Flags: needinfo?(jrburke)
(Assignee)

Comment 5

2 years ago
Thanks for the help here James!

(In reply to James Burke [:jrburke] from comment #4)
> I am concerned about how shared/js/component_utils is included

One option could be to potentially create a module to handle the loading of this dependency in the e-mail app.


> I also wish there was a way to optimize the style scoped @import usage for
> the stylesheets. As it is now, they cannot be optimized by including it with
> other style sheets loaded on startup, and so I am concerned longer term
> around our startup pages using many custom elements in this style.

Valid concern. Sounds like we should definitely do some performance measurements here. So far we've been measuring startup time, and haven't found regressions. Memory measurement would be a good next step.


> I also feel like the custom elements stuff is still in flux, and given the
> lack of a clear UX refresh for 2.5 also not seeing an immediate need for the
> change.

We're going to try to implement custom elements everywhere in 2.5 - product, ux, and many engineers are asking for it. If it's a no-go for e-mail, we could potentially host a local copy of building blocks inside of the e-mail app. These building blocks are most likely being discontinued in 2.5/3.0, and will be removed from the shared/ folder.
> One option could be to potentially create a module to handle the loading of this dependency in the e-mail app.

> Valid concern. Sounds like we should definitely do some performance measurements here. So far we've been measuring startup time, and haven't found regressions. Memory measurement would be a good next step.

For both of these, I feel like we know how these things turn out given the 1.0 experience: they have problems. No explicit dependency relationships lead manual ordering of dependencies and bugs, and we know bundling of assets makes a performance difference. It would be great to capitalize on that experience in any new efforts.

However, I appreciate it is difficult when trying to deal with platform constraints.

> We're going to try to implement custom elements everywhere in 2.5 - product, ux, and many engineers are asking for it. If it's a no-go for e-mail, we could potentially host a local copy of building blocks inside of the e-mail app. These building blocks are most likely being discontinued in 2.5/3.0, and will be removed from the shared/ folder.

If they are truly going away then email will likely go along with whatever ends up being the solution. My desire is to do the cutover as far down the line as possible though, since I still believe the story is not solid on custom elements, and the existing pass for gaia components feels like it could use another pass. Also, given the 2.5 priorities, there is nothing about a UI refresh that would require doing the cutover.

I'm also feeling a bit burned lately by the platform sand we are building on, which leads to bugs like bug 1182358 and bug 1176829, where I spend more of my time debugging and dealing with those issues than solving actual user needs in app-space. Introducing more code that is subject to more of that sand is not appetizing at the moment. Hopefully I will be more receptive once I can make some progress on the user needs for dogfooding. I appreciate your desire for common building blocks and jumping in to meet that goal though.
I'm with James here on all three points:

1) I love the work that :kgrandon and :wilsonpage are team are doing to try and move web standards forward as it relates to web-components, etc.  And we understand the benefit of trying to dogfood these new technologies, as evidenced by email's adoption of document.registerElement as a baby-step.

2) Dogfooding these new technologies and platform advances only works out okay if platform problems are addressed in a timely fashion and we have the cycles to deal with the fallout.  And it requires app developers/us to have cycles to investigate those problems, which means that we need to not already be overwhelmed by other application-domain issues.  The platform team generally seems overwhelmed and bandwidth limited, as evidenced by IndexedDB-on-workers-from-inside-promises resulting in cycle collector crashes (bug 1152026) having been an issue for over 3.5 months with the apparent fix not getting reviewed until next week at the earlier, the custom element inconsistency bug 1176829 showing no progress after going on 2 weeks, DOM Workers being unreliable in launching for 2.5 months (bug 1119157), etc.  The prospect of involving the shadow DOM as another moving part for no functional benefit worries me a lot.

3) I want to make an email app that is reliable and people can use.  (Or at least makes the foxfooders complain less! ;)  We've leveraged the v2.5 breather to undertake an overhaul of the email app so it thinks in conversations, and address some of the infrastructure issues that have led to email wackiness after sustained use.  But there is a tremendous amount of work to still be done and likely a tremendous amount of fallout that we'll have to deal with.  And with us down mcav again, we really don't have time to be worrying about the gaia components and potential breakage.  (Especially the lack of explicit dependencies again.  Looking at the components, I see they're using innerHTML which means they're going to grow a dependency on :freddyb's innerHTML helper library, which means more dependencies can and will come, etc.)


So I agree that if the plan is to remove the building blocks or change them in non-trivial ways that we should just duplicate the code into email until such time as email's UI goes wildly out-of-spec, we have the engineering resources to do a changeover, and/or we're already overhauling the UI's in question.  Given that we hope to have conversations golden in 2.5 and we ideally will get some headcount by the time 2.5 ships, this seems potentially feasible in post-2.5.  Especially if the more daring/heroic applications have already shaken all the bugs out already!
You need to log in before you can comment on or make changes to this bug.