Add section for transformation settings

[pestevez]

This PR adds a new section to define additional settings for the transformation.

Here is a summary of the changes:

• Use an accordion to separate the existing rules configuration from the new settings
• Add a new accordion pane to define:
  • Style name
  • Audience Network Placement ID
  • Ads raw HTML
  • FB Pixel ID
  • Analytics raw HTML
• Upgrade the minimum SDK version used for the preview, so that the new fields are processed and included in the generated IA markup
• Add unit tests

Testing Plan

• Upgrade the IA SDK version in the webserver folder by running composer install
• Load a sample article
• Fill out the required selectors
• Switch to the new settings section and specify
  • Style Name
    • Verify the new name appears in the generated markup
  • FB Pixel ID
    • Verify the ID appears in the generated markup
  • Audience Network Placement ID
    • Verify the ID appears in the generated markup
• Clear the FB Pixel ID and the Audience Network Placement ID fields
• Set values for both of the raw HTML fields
  • Verify the values are in the generated markup

[mburak]

I think that we should change the Ads section to be a select box where you can select "Audience Network" or "Other/Raw HTML" and based on the selection we show the text/area field, because you shouldn't be able to add both. Thoughts?

[pestevez]

Makes sense, @mburak. I will make that change today. By the way, the same logic will not apply to the Analytics section (which you did not mention, but I am clarifying) as you can have multiple trackers in the same Instant Article.

Also, does it make sense to bump up the version in the package.json file to 0.0.2?

[mburak]

Yes, I didn't ask about Analytics section because the behavior is correct.
Yes we can bump it, but maybe to 0.1.0 as we are adding new functionality?

[pestevez]

Here is the updated version, @mburak. These are the new changes:

• Updated version to 0.2.0
• Added logic to handle ads (None, Audience Network, or Raw HTML), including unit tests
• Added logic to show the settings fields in the UI after importing from a file

I found an issue with the IA SDK where if all the fields in the ads settings object are not included an exception is thrown.

Please let me know if you need anything else.

[mburak]

It looks great @pestevez! Just a small style nit. As you can see in this picture, the Type label is missing the bullet:

Let's add it to make it consistent.

Thanks for pointing out that SDK issue. We were aware of it but we never fixed it. I think it's of easy resolution. (cc @everton-rosario )
Let's wait for a new SDK version with this fix and use that one here.

[pestevez]

Thanks, @mburak. For the label's bullet, I feel this field is a bit different. We have been using the following icons:

• Bullet point for optional fields with no value
• ✘ for required fields with no value
• ✔ for fields with value (required and optional)

In this case, the drop-down itself is not considered required or optional (it's like Schrödinger's cat!) because it only controls which fields are shown, and these fields are the ones that are optional or required (all optional in this case.)

I feel the "Type" label is similar to the one we have for selector or attribute, that do not have an icon:

but I am happy to add the icon, if you feel strongly about it.

In any case, I will hold on merging this PR until the SDK issue is resolved.

[mburak]

Oh now I get it! As every field was using the bullet point, I thought every field was using it, that's why it looked weird to me. Now I get your point. Perhaps we could add some help tips indicating that what you explained in the last comment?

[pestevez]

I updated the version of the IA SDK used by the preview webserver. I also created #85 to track @mburak's request to add some help tips.

[mburak]

Great stuff @pestevez! I just added a comment, feel free to work on that here or open a new issue for that.
Thanks!

[mburak]

I think we should have constants for these values ✔and • as we are using them in a lot of places. Or maybe a helper function where you pass the value and it renders one or the other based on if it's null or not.
As this is just a general improvement, feel free to open a new issue to address this.

[pestevez]

Sounds good! Thanks