Skip to content

Fix LT-22187: Allow reordering fields#435

Merged
jtmaxwell3 merged 7 commits into
release/9.3from
LT-22187
Aug 13, 2025
Merged

Fix LT-22187: Allow reordering fields#435
jtmaxwell3 merged 7 commits into
release/9.3from
LT-22187

Conversation

@jtmaxwell3

@jtmaxwell3 jtmaxwell3 commented Aug 12, 2025

Copy link
Copy Markdown
Collaborator

This fixes https://jira.sil.org/browse/LT-22187. When you move a field, it moves it after the part for the next slice. For this to work, you have to get the right part for the slice (the one that can be moved).


This change is Reviewable

@jasonleenaylor jasonleenaylor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, all discussions resolved


DistFiles/Language Explorer/Configuration/Parts/LexEntry.fwlayout line 7 at r1 (raw file):

	<layout class="LexEntry" type="detail" name="Normal">
		<part ref="ChangeHandler"/>
		<part label="Lexeme Form" ref="LexemeForm"/>

Was removing the label necessary?


Src/Common/Controls/DetailControls/Slice.cs line 2941 at r1 (raw file):

			// This is the first node in a (possibly singleton) sequence.
			// Does it represent itself (and so can be moved) or the sequence as a whole (and so can't be moved at this level)?
			// Look at the reference part nodes above node to determine.

This was a good effort and describing what we are trying to determine. I'm trying to decide what kind of comments/refactors will help us fix any future bugs here or avoid breaking this if we need to modify it for another reason.


Src/Common/Controls/DetailControls/Slice.cs line 2943 at r1 (raw file):

			// Look at the reference part nodes above node to determine.
			bool found = false;
			string label = XmlUtils.GetOptionalAttributeValue(node, "label", null);

I don't see this label being used anywhere.

@jtmaxwell3

Copy link
Copy Markdown
Collaborator Author

Removing the Lexeme Form label was necessary. It was redundant with the label in AsLexemeForm, and it made the code think that there was a header above AsLexemeForm that could be moved, something like:

Lexeme Form
Lexeme Form
Morph Type

Then the code determined that AsLexemeForm could not be move because it had no siblings.
Deleting the higher Lexeme Form allowed AsLexemeForm to be move.

If we don't want Lexeme Form to be moved, then we can restore the higher Lexeme Form label.

@jasonleenaylor jasonleenaylor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 2 files at r1, 1 of 2 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jtmaxwell3)

@jtmaxwell3 jtmaxwell3 enabled auto-merge (squash) August 13, 2025 15:43
@jtmaxwell3 jtmaxwell3 merged commit 338e882 into release/9.3 Aug 13, 2025
5 checks passed
@jtmaxwell3 jtmaxwell3 deleted the LT-22187 branch August 13, 2025 16:33
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.

2 participants