Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Structure tree should show image number (or name), not internal order number #4038

Closed
matthias-ronge opened this issue Oct 8, 2020 · 19 comments · Fixed by #4642
Closed

Structure tree should show image number (or name), not internal order number #4038

matthias-ronge opened this issue Oct 8, 2020 · 19 comments · Fixed by #4642

Comments

@matthias-ronge
Copy link
Collaborator

In the example, pages 2-6 are to be added to the new structure element.

Screenshot 1

Screenshot 2

The number displayed has changed. Picture 2 is now picture 1, and picture 1 is now picture 6. In short, you cannot use the (changing) numbers as a guide to identify the index of the scanned picture (and the continuous book, from front to back).

Screenshot 3

Goal: In front of the colon, the canonical part of the file name without leading zeros should be displayed, not the internal order number. Then you would always know where you are.

@andre-hohmann
Copy link
Collaborator

I strongly support this suggestion. The current behaviour is very confusing.
From my point of view, the combined structure will be much more usable if Matthias goal is reached.

@IkramMaalej
Copy link
Collaborator

@subhhwendt The internal order number shouldn't be displayed only in the gallery panel or also in structure data panel?

@BartChris
Copy link
Collaborator

BartChris commented May 10, 2021

@matthias-ronge

We also encountered this behaviour. I made some internal changes to display the image names in the structure tree. However i am not sure in which places the image name should be displayed. Apart from that i am no expert in JSF so i am hesitating to make a pull request here. So here just for reference the changes i did. Hopefully someone can adress this soon in the master because it really improves the editing experience:

Changes in StructurePanel.java:

old:

    private String buildViewLabel(View view) {
        String order = view.getMediaUnit().getOrder() + " : ";
        if (Objects.nonNull(view.getMediaUnit().getOrderlabel())) {
            return order + view.getMediaUnit().getOrderlabel();
        } else {
            return order + "uncounted";
        }
    }

new:

import org.apache.commons.io.FilenameUtils;

private String buildViewLabel(View view) {
        URI uri = view.getMediaUnit().getMediaFiles().entrySet().iterator().next().getValue();
        String fileName = FilenameUtils.getBaseName(uri.getPath()) + " : ";
        if (Objects.nonNull(view.getMediaUnit().getOrderlabel())) {
            return fileName + view.getMediaUnit().getOrderlabel();
        } else {
            return fileName + "uncounted";
        }
    }

Result:

grafik

This also brings up the question on wether "advanced" mode of the metadata editor should be adapted, too. Especially given the fact that there are difference in functionality between the two editing modes:

#4380

@matthias-ronge
Copy link
Collaborator Author

Yes, the thing with the “canonical part of the file name” is a bit more difficult, but thank you for your work. Can you test if this works?

private String buildViewLabel(View view) {
    MediaUnit mediaUnit = view.getMediaUnit();
    Entry<MediaVariant, URI> mediaFileEntry = mediaUnit.getMediaFiles().entrySet().iterator().next();
    final String use = mediaFileEntry.getKey().getUse();
    Subfolder subfolder = new Subfolder(dataEditor.getProcess(), dataEditor.getProcess().getProject().getFolders()
            .parallelStream().filter(folder -> folder.getFileGroup().equals(use)).findAny().get());
    String canonical = subfolder.getCanonical(mediaFileEntry.getValue());
    return Objects.nonNull(mediaUnit.getOrderlabel()) ? canonical + ':' + mediaUnit.getOrderlabel() : canonical;
}

If so, you can make a pull request

@BartChris
Copy link
Collaborator

It seems to work. I issued a pull request.

@solth
Copy link
Member

solth commented May 11, 2021

I am not sure the proposed changes are correct. It seems there is a misunderstanding about how pages - and structures, for that matter - are ordered in Kitodo 3.

AFAIK, the filename determined the physical order of pages in Kitodo 2. In that case displaying the canonical part of the filenames makes sense. In Kitodo 3 however, the ordering of pages and structure elements is solely defined by the ORDER attribute of the elements within the METS structure (which allows for arbitrary filenames where one does not have to worry about impacting page order by accident). This attribute is initially determined by the sorting of the filenames in the files system (which - I think - can vary between operating systems and therefor isn't a good indicator for physical page order anyway).

Whenever the user moves a structure element (or individual pages) however, the ORDER attribute (and the position of the corresponding mets:div elements in the physical structure map as well as the order of smLinks) is updated accordingly, representing the new physical order of the pages. Therefor, the filenames are completely irrelevant when visualizing the order of pages in a document (apart from the first time a process is loaded and saved in the metadata editor), and the proposed changes would falsely imply otherwise.

I think the problem stems mainly from the current position options for new structure elements being limited to "First child", "Last child", "Previous sibling" and "Next sibling", all of which implicitly result in assigned pages being moved correspondingly and therefor receiving new ORDER attributes.

IMO, the changes in #4365 will greatly reduce the confusion, because they allow creating new structure elements from selected pages in their "Current position" without reordering the pages.

@BartChris
Copy link
Collaborator

BartChris commented May 11, 2021

Thank you very much for the detailed explanation. Since the other options for adding structure elements would still be there after your changes are merged: would it maybe be possible to show both- the ordernumber AND the original image name? I do not know how the display of the Information would look like, but that would reflect the internal Kitodo structure and also avoid confusion on the user's side as outlined above.

@solth
Copy link
Member

solth commented May 11, 2021

Yes, that should be possible, and I think there is already an issue that might cover this: #4202

@matthias-ronge
Copy link
Collaborator Author

The solution is the right one for the problem described. What you describe, @solth, is true in theory, but in practice there is another implication: For a user, a page has two properties, the number of the scan from the file name, and the pagination. The number from the file name corresponds to the physical sequence, except for later scanned corrections which may have a higher number than the image of the back cover, which is typically easy to find because it is optically very different (often with a color checker). The filename (number) is very important for the user to see, because only from it he can notice whether he has accidentally dragged pages into the wrong order, if the physical medium is not available for him when structuring and the book has no pagination in this page range. The internal order number is irrelevant for the user to see. It is always correctly assigned by Production, and the user can tell the current order of the images from looking on them.

#4202 is no replacement for this requirement.

@solth
Copy link
Member

solth commented May 12, 2021

@matthias-ronge Yes, would you describe is true for Kitodo 2, but not for Kitodo 3, if I understand it correctly. That's what I tried to explain in my previous comment: the "internal" ORDER is the physical order of the pages, no matter how the files are named! That's why it is correct to display the ORDER and update it on drag'n'drop operations (which weren't available in Kitodo 2, IIRC?).

The internal order number is irrelevant for the user to see. It is always correctly assigned by Production, and the user can tell the current order of the images from looking on them.

That's what I thought initially, too, "the pages are just continuously numbered in the tree and therefore the number does not need to be displayed". But after it became obvious that the order of pages and sibling structures is not easily visible in the gallery alone (see #4368 ), the relevance of displaying the ORDER - at least in the gallery - became a little clearer to me. Without it, there is currently no way to tell if there is a structure element between two pages on the same level when just using the gallery.

I have a suspicion this topic might go a little deeper and is a conceptional issue. Would you mind if I transform this issue into a Github 'Discussion' so we could discuss it before coming to a conclussion what the real issue is that needs to be addressed?

@andre-hohmann
Copy link
Collaborator

I want to support @matthias-ronge point of view in the comment before, especially:

The filename (number) is very important for the user to see, because only from it he can notice whether he has accidentally dragged pages into the wrong order, if the physical medium is not available for him when structuring and the book has no pagination in this page range.

As i only work without the original resources, it happend several times, that i could not clearly identify pages without pagination, because the number in the structure tree changed as soon as new structure elements were created, pages were moved, ...
If each image had a pagination number, it would work and often, i did it to ensure that the order and the assignment to structure element is correct. It is difficult to explain, but if you try to structure a process without the original and you have several pages without pagination, you will quickly understand the problem.

@solth suggestion to open a new issue for the underlying problem is very good. I think that is the problem, which @sebastian-meyer pointed out, when we discussed about ORDER, ORDERLABEL, ...

@solth
Copy link
Member

solth commented May 12, 2021

I strongly support this suggestion. The current behaviour is very confusing.

@andre-hohmann could you test the changes in #4365 ? I would hope that adding structure elements and assigning selected pages would be much less confusing with those changes, too. I think @subhhwendt already tested those changes successfully.

Additionally, I would suggest to deactivate all other positions at which a new structure element can be inserted - "First child", "Last child", "Previous sibling", "Next sibling" - when creating a new structure element from selected pages, so that structure elements from pages can only be created "in place", in order to avoid implicit re-ordering of those selected pages. Could that further ease the pain?

@andre-hohmann
Copy link
Collaborator

It seems as if the order-number is still active in:

  • pagination-section
  • adding a new structure element

Is this wanted that way, @solth? The colleagues are confused by this behaviour and i would prefer a uniform labeling of the images, too.

PaginierungAbweichung02

@andre-hohmann andre-hohmann reopened this Oct 11, 2021
@solth
Copy link
Member

solth commented Oct 11, 2021

@andre-hohmann The ORDER attribute is used internally on structure elements to be able to collectively sort/order pages and structure elements within another structure element.

@oliver-stoehr I think you implemented this unified order feature, can you say if it will still work with the adjustments @andre-hohmann requested?

@andre-hohmann
Copy link
Collaborator

To avoid misunderstandings:

  • I understand the function of the ORDER attribute and the behaviour in the structure tree is now absolutely fine!
  • I am only confused about the differing labeling in the editors. In my opinion, the labeling of the structure tree should be applied in all editors - if this is possible. Otherwise i will propose the colleagues to use only the gallery for structuring.

@oliver-stoehr
Copy link
Collaborator

It should be absolutely fine to display the same label in the pagination section and "new element" popup as in the structure panel. If I understand the request of @andre-hohmann correctly there is no need to change the ORDER attribute and ordering of pages.

@andre-hohmann
Copy link
Collaborator

there is no need to change the ORDER attribute and ordering of pages.

No, the behaviour in the structure tree is correct. The ORDER attribute should not be depicted as image-number.
If in the example above, in all editors or popups the following siblings are shown, i am happy:

  • 3:1 (Order = 1)
  • 4:4 (Order = 2)
  • 1:2 (Order = 3)
  • 2:3 (Order = 4)
  • 5:5 (Order = 5)
  • 6:6 (Order = 6)

The ORDER attribute should only be used internally to allow reordering the images without changing the filename, et cetera. The user can derive the ORDER number from the order of the files from the order in the structure tree. In my opinion, there is no need to show it.

I just hope, we really understand reach other - that is quite tricky ;-)
Maybe @matthias-ronge can have a look on it, to make sure that everything is correct. He also opened the issue.

@matthias-ronge
Copy link
Collaborator Author

The label is correctly formed and displayed in the structure tree since the pull request #4642. The same should be done here (call DataEditorForm.getStructurePanel().buildViewLabel(View) to get the correct label).

@andre-hohmann
Copy link
Collaborator

I opened a new issue for the problem which is described in #4038 (comment).

I close this issue, because the original problem described by @matthias-ronge has been solved.
The new "problem" should be analysed separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment