Skip to content

Removed reorganizing verse numbers automatically#458

Merged
tjcouch-sil merged 3 commits intomainfrom
pt-3379-stop-reorganizing-verses
Mar 9, 2026
Merged

Removed reorganizing verse numbers automatically#458
tjcouch-sil merged 3 commits intomainfrom
pt-3379-stop-reorganizing-verses

Conversation

@tjcouch-sil
Copy link
Copy Markdown
Contributor

@tjcouch-sil tjcouch-sil commented Mar 5, 2026

Also explained nx extract-api so I don't have to go find Ira's note about it on my previous PR every time I make changes ;)

I don't know if this functionality is still desired in some editor usage contexts outside of Platform.Bible; let me know if you'd like me to add a prop so we can just turn this off in Platform.Bible and leave it otherwise.


Open with Devin

This change is Reviewable

@codesandbox
Copy link
Copy Markdown

codesandbox bot commented Mar 5, 2026

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@jolierabideau jolierabideau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just one question.

irahopkinson
irahopkinson previously approved these changes Mar 6, 2026
Copy link
Copy Markdown
Collaborator

@irahopkinson irahopkinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM but consider 2 posts.

Copy link
Copy Markdown
Collaborator

@irahopkinson irahopkinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@irahopkinson reviewed 5 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on tjcouch-sil).


libs/shared-react/src/plugins/usj/UsjNodesMenuPlugin.tsx line 45 at r1 (raw file):

  const [editor] = useLexicalComposerContext();
  useVerseCreated(editor);

NIT you could have parametrized this and left the code in so platform-editor could just turn it off by default. However this is git so the code isn't lost.

Code quote:

  const [editor] = useLexicalComposerContext();
  useVerseCreated(editor);

Copy link
Copy Markdown
Contributor Author

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tjcouch-sil made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on irahopkinson and jolierabideau).


libs/shared-react/src/plugins/usj/UsjNodesMenuPlugin.tsx line 45 at r1 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

NIT you could have parametrized this and left the code in so platform-editor could just turn it off by default. However this is git so the code isn't lost.

It was already partially broken (copy/paste didn't work right as detailed in the work item, and my testing seemed to indicate making verse numbers didn't work quite right in some contexts), so parameterizing it doesn't really feel that useful in my mind unless someone else actually wants to use and maintain it as I mentioned in my PR description. Felt like a YAGNI.

Let me know, and I can add it back in exactly as it was if you want.

irahopkinson
irahopkinson previously approved these changes Mar 9, 2026
Copy link
Copy Markdown
Collaborator

@irahopkinson irahopkinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: thanks for this.

@irahopkinson reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on jolierabideau).

jolierabideau
jolierabideau previously approved these changes Mar 9, 2026
Copy link
Copy Markdown
Contributor

@jolierabideau jolierabideau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jolierabideau reviewed all commit messages and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on tjcouch-sil).

@tjcouch-sil tjcouch-sil dismissed stale reviews from jolierabideau and irahopkinson via a4e7537 March 9, 2026 21:15
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 6 additional findings in Devin Review.

Open in Devin Review

@tjcouch-sil tjcouch-sil force-pushed the pt-3379-stop-reorganizing-verses branch from a4e7537 to c11755b Compare March 9, 2026 21:23
@tjcouch-sil tjcouch-sil force-pushed the pt-3379-stop-reorganizing-verses branch from c11755b to bc4eac7 Compare March 9, 2026 21:23
Copy link
Copy Markdown
Collaborator

@irahopkinson irahopkinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@irahopkinson reviewed 1 file and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on tjcouch-sil).

Copy link
Copy Markdown
Contributor Author

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tjcouch-sil resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on tjcouch-sil).

@tjcouch-sil tjcouch-sil merged commit 4e5f73b into main Mar 9, 2026
7 checks passed
@tjcouch-sil tjcouch-sil deleted the pt-3379-stop-reorganizing-verses branch March 9, 2026 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants