guardian / scribe-plugin-noting

9 stars 3 forks source link

Configurable tags #68

Closed cutandpastey closed 9 years ago

cutandpastey commented 9 years ago

This PR adds the ability to construct notes of different tags i.e <gu-flag></gu-flag>.

QA

Test all note functionality on the example page using F9, ensure gu-flag tags are constructed.

robinedman commented 9 years ago

Is this supposed to be a first step or a more complete "flag & correct" thing? Would help to know when reviewing it. If it's literally what the description says it looks good to me :) but otherwise I can immediately see some issues!

Such as:

cutandpastey commented 9 years ago

Iv'e added tests and fixes for note collision, so if @robinedman is happy can we merge?

robinedman commented 9 years ago

It's still possible to create a note within a flag and vice versa so I'm not sure if the collision stuff is working.

A couple of more things:

cutandpastey commented 9 years ago

@robinedman note collision should now be fixed.

robinedman commented 9 years ago

Can't test this atm. The build fix broke the build (https://github.com/guardian/scribe-plugin-noting/commit/10489c556148864a8ae071ab2b470dd6451cac36).

Now npm run build doesn't build the JS anymore for some reason.

cutandpastey commented 9 years ago

Fixed

robinedman commented 9 years ago

Works really well now! :+1:

cutandpastey commented 9 years ago

Cool, I'll wait for travis before releasing.

cutandpastey commented 9 years ago

Im reverting this to get the fix for https://github.com/guardian/scribe-plugin-noting/issues/75 out