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

Remove usages of innerHTML where possible #11545

Closed
3 tasks done
newmuis opened this issue Oct 3, 2017 · 1 comment
Closed
3 tasks done

Remove usages of innerHTML where possible #11545

newmuis opened this issue Oct 3, 2017 · 1 comment
Assignees
@alanorozco
Labels
WG: stories
Milestone
Pending Triage

Comments

@newmuis
Copy link
Contributor

newmuis commented Oct 3, 2017 •
edited by alanorozco
Loading

We're currently using innerHTML in some places in the amp-story extension; we should instead use createElement/appendChild to mitigate future risks where we move from using static templates to dynamic ones including publisher-specified data.

  • SVG icons (Use background image icons in amp-story #11610)
  • extensions/amp-story/0.1/amp-story-page.js: loadingScreen./*OK*/innerHTML = LOADING_SCREEN_CONTENTS_TEMPLATE;
  • extensions/amp-story/0.1/system-layer.js: this.root_./*OK*/innerHTML = TEMPLATE;
@newmuis newmuis mentioned this issue Oct 3, 2017
Merged
@dreamofabear dreamofabear mentioned this issue Oct 3, 2017
Merged
@dvoytenko
Copy link
Contributor

A big group of these are <svg> icons. We should just try to move them all via CSS.

@newmuis newmuis changed the title Migrate amp-story internal markup to use DOM APIs instead of innerHTML Remove usages of innerHTML where possible Oct 6, 2017
@newmuis newmuis mentioned this issue Oct 6, 2017
Merged
@alanorozco alanorozco mentioned this issue Oct 9, 2017
Merged
@ampprojectbot ampprojectbot added this to the Pending Triage milestone Oct 9, 2017
@alanorozco alanorozco mentioned this issue Oct 10, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alanorozco alanorozco

Labels
WG: stories
Projects
None yet
Milestone
Pending Triage
Development

No branches or pull requests
4 participants
@alanorozco @dvoytenko @newmuis @ampprojectbot