This repository has been archived by the owner on Sep 2, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 5
First pass at 'everything demo' of transforms on actual articles #164
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@montehurd This looks great. Just one question: Is the order the transforms show up in the control in the same order as they should be invoked? If not I think that would be useful. Other than this question this looks great. |
moving all the article html construction to the article ref to make it easier to extend article ref to handle full page PCS content in a transparent way
`base.styles` has html and body tag height at 100% for some reason
berndsi
approved these changes
Apr 24, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thank you very much for your work on this!
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://phabricator.wikimedia.org/T172166
This first pass implements the following:
It still needs to conform to our linting and documentation standards, which I'll chip away at.
@berndsi and I also discussed updates which will need to be made to support representation of
PCS
article data. Things we discussed:articleRef
support non-section loading (PCS delivers entire article, not sections - move index html and section html wrapping intoarticleRef
?) and related changes to the demo html filea way to exclude particular transforms from particular sources (for example, PCS already does certain transforms, MCS already relocates 1st paragraphs, etc...)(Will look at later)what we could show above each article preview iframe to indicate what transforms were actually applied to that article (PCS for example wouldn't list any transforms it applies upstream) to make proofing expectations easier to confirm(Will look at later)Other considerations: