search

guardian / scribe-plugin-noting

9 stars 3 forks source link

Misleading function names (findFirstNoteSegmentBelow -> findNextNoteSegment, findFirstNoteSegmentAbove -> findPrevNoteSegment ?) #41

Closed robinedman closed 9 years ago

robinedman commented 9 years ago

The functions findFirstNoteSegmentAbove and findFirstNoteSegmentBelow don't do what their names say they do. Better names would be something like findPreviousNoteSegment and findNextNoteSegment.

Or if they should actually be finding the first note above/below, then they need to do focus.find(isNote, 'up') and focus.find(isNote, 'down').

See: https://github.com/guardian/scribe-plugin-noting/blob/jp-refactor-note-toggle-2/src/utils/noting/find-next-note-segment.js#L10

And: https://github.com/guardian/scribe-plugin-noting/blob/jp-refactor-note-toggle-2/src/utils/noting/find-previous-note-segment.js#L10

cutandpastey commented 9 years ago

Fixed with https://github.com/guardian/scribe-plugin-noting/pull/58