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

remove obsolete label element in metadata-schema.component.html #2505

Merged
merged 2 commits into from
Sep 22, 2023

Conversation

saschaszott
Copy link
Contributor

@saschaszott saschaszott commented Sep 19, 2023

This PR implements a minor HTML change which improves the "find in page" of your prefered web browser. Currently, it is not possible to search for metadata field names of the form schema.element.qualifier using the "find in page" function, e.g. in /admin/registries/metadata/dc.

@tdonohue tdonohue added bug 1 APPROVAL pull request only requires a single approval to merge port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release labels Sep 19, 2023
@alanorth
Copy link
Contributor

Thanks @saschaszott. If I understand correctly, by "find in page" you mean the Ctrl-F / Cmd-F in the browser? I tried searching for dc.contr using Ctrl-F in Firefox on DSpace 7.6 and it works.

Did I understand correctly?

@saschaszott
Copy link
Contributor Author

saschaszott commented Sep 20, 2023

@alanorth , the problem only arises if you search with a field name that contains two periods.

This screenshot was taken from Google Chrome where a search with term dc.contributor.advisor does not match the page content.

image

@alanorth
Copy link
Contributor

OK, I don't use Chrome so I don't know. I can find a metadata field with two dots in Firefox.

Is this because it's wrapping for some reason in Chrome, or perhaps just on your device?

@saschaszott
Copy link
Contributor Author

@alanorth , I cannot reproduce this odd behaviour in Safari. This seems to be a Chrome specific problem. Nevertheless, it should be fixed as Chrome has > 50% market coverage.

Copy link
Contributor

@alanorth alanorth left a comment

Choose a reason for hiding this comment

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

This introduces trailing whitespace.

@alanorth
Copy link
Contributor

@saschaszott I finally looked at this. The existing code doesn't make sense—why put a label with only part of the metadata field? Could have been a bug in the original code.

Anyways, I tested this and it doesn't break my find in Firefox so I'm +1. Would you feel comfortable rebasing this commit to remove the trailing whitespace? Thanks.

@saschaszott
Copy link
Contributor Author

This introduces trailing whitespace.

Thank you for noticing - it was fixed.

@alanorth
Copy link
Contributor

Thanks @saschaszott. I will squash the two commits into one before merging so the whitespace doesn't stay in git history.

@alanorth alanorth merged commit 77d6212 into DSpace:main Sep 22, 2023
10 checks passed
@dspace-bot
Copy link
Contributor

Successfully created backport PR for dspace-7_x:

@tdonohue tdonohue added this to the 8.0 milestone Sep 22, 2023
@tdonohue tdonohue removed the port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release label Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge bug
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants