search

guardian / scribe-plugin-noting

9 stars 3 forks source link

Can now select entire flag and correct #122

Closed robinedman closed 9 years ago

robinedman commented 9 years ago

Fixes https://github.com/guardian/scribe-plugin-noting/issues/120

We were...

Edit: see https://github.com/guardian/scribe-plugin-noting/commit/5b17a03d7f0217c81f6425005c8439e99b3c16fa for the core issue that caused this bug.

cutandpastey commented 9 years ago

I think there is more work to be done see: https://github.com/guardian/scribe-plugin-noting/blob/re-select-whole-flag-correct/src/generate-note-controller.js#L130 which does not pass tagName.

robinedman commented 9 years ago

Ok. What do you think about removing the defaults and making tagName mandatory everywhere? It's easy to get this wrong.

cutandpastey commented 9 years ago

Dunno, the intention was to reduce errors by falling back to gu-note but in practice this ends up causing more problems than it solves.

robinedman commented 9 years ago

Let's not do it as part of this PR. I created an issue: #123

robinedman commented 9 years ago

That we don't pass a tagName to this.selectNote is correct though. That function will iterate through the all the tag names.

cutandpastey commented 9 years ago

:+1:

cutandpastey commented 9 years ago

:+1: I like that you have taken to using the VFocus utils!