Refactor of article code #17
Open
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
<A> tags don't have a src attribute. This should have been href instead.
The original felt a little out of place. I wanted to encapsulate all the moving parts into intention revealing names and loose the awkward switch conditional (which doesn't scale well if new tag cleanup algorithms were to be introduced). Using polymorphism helps place the implementation of cleaning specific types in an encapsulating class.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I liked the idea that thie article parsing was doing. how it used an iframe to parse the HTML string into DOM and allow the browser to sanitize it before giving it back to Ember. I thought this was a clever way to use Ember client side to parse HTML without the need of a server side proxy.
The code itself bothered me in how it was large functions doing a lot of things with a switch statement driving it all. So I thought it would be a good thought exercise to see how some OOP design practises could make this different.
There was also a bug in the cleaning of an
<A>tag. So I split these into two commits. If you felt the refactoring was not a good idea the bug fix can be cherry-picked by itself.Also, I attempted to test this on my own machine and nothgin worked. My environment is in direct opposition with Ember
1.10(which at this point is super old!). So hopefully you have a dev environment that actually builds to try it out!