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

Strip leading and trailing spaces from most/all fields #10130

Closed
zorae opened this issue Dec 7, 2024 · 11 comments · Fixed by #10249
Closed

Strip leading and trailing spaces from most/all fields #10130

zorae opened this issue Dec 7, 2024 · 11 comments · Fixed by #10249
Assignees
Labels
Lead: @scottbarnes Issues overseen by Scott (Community Imports) Priority: 3 Issues that we can consider at our leisure. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed]

Comments

@zorae
Copy link

zorae commented Dec 7, 2024

Proposal

When editing an author, work, or edition, invisible characters such as spaces and tabs should be automatically stripped from the beginning and end of a field.

Justification

Problem: Librarians may accidentally include leading/trailing spaces, which – if not manually removed – are saved to the database. This can lead to e.g. author names looking weird.

Impact: Librarians can save time, e.g. when pasting names from external sites.

Note: We would have to check for edge cases in which leading/trailing spaces have a legitimate use and exclude those fields.

Breakdown

Requirements Checklist

  • [ ]

Related files

Stakeholders


Instructions for Contributors

Please run these commands to ensure your repository is up to date before creating a new branch to work on this issue and each time after pushing code to Github, because the pre-commit bot may add commits to your PRs upstream.

@zorae zorae added Needs: Breakdown This big issue needs a checklist or subissues to describe a breakdown of work. [managed] Needs: Lead Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed] labels Dec 7, 2024
@QuangPhung15
Copy link
Contributor

Hi @zorae,

I want to work on this issue. It is a useful improvement to streamline data entry and prevent formatting errors. I'll review potential edge cases where leading/trailing spaces might be necessary and ensure the implementation excludes those fields.

Please let me know if this feature has any additional details or preferences!

@zorae
Copy link
Author

zorae commented Dec 10, 2024

Thanks @QuangPhung15!
I’ve been pondering if spaces should a) be stripped immediately upon deselecting a field, or b) only upon saving changes.

a) has the advantage that it gives librarians immediate feedback so they know they don‘t have to manually remove spaces. But I foresee that this could be annoying for multi-line fields like bio or work description where a librarian might start writing something, add a space or line break at the end and deselect the field before coming back to continue writing.

Option a) would have to be implemented client-side, while b) could be either client- or server-side.

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Dec 11, 2024
@bitnapper
Copy link

Would it also be possible to substitute multiple space characters to only one, something like: s/\S+/ /g?

@mekarpeles
Copy link
Member

mekarpeles commented Dec 16, 2024

@zorae can we please be enumerative of the exact fields you believe this should be applied to? Even if we start with only a few, that is more likely to make forward progress than requiring folks who don't have librarian context / experience from having to figure out what fields we might be talking about. Thank you!

Also, as @scottbarnes mentioned to me in a passing, there may be edge cases, like author, where there are things like ", Sr." or other prefixes that end up complicating these rules.

@mekarpeles mekarpeles added Priority: 3 Issues that we can consider at our leisure. [managed] Lead: @scottbarnes Issues overseen by Scott (Community Imports) and removed Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Needs: Lead labels Dec 16, 2024
@zorae
Copy link
Author

zorae commented Dec 18, 2024

@mekarpeles Some fields I would find particularly useful are author name, bio, date of birth, date of death, edition title, work title, and work description.

@scottbarnes Can you clarify what the issue would be with the “, Sr.” suffix? It ends with a period, so I don’t see how a rule that strips out whitespace at the end of a field would affect it.

@scottbarnes
Copy link
Collaborator

I apologize, I think I misheard this when the issue came up as including the stripping of periods.

@scottbarnes scottbarnes removed Needs: Response Issues which require feedback from lead Needs: Breakdown This big issue needs a checklist or subissues to describe a breakdown of work. [managed] labels Dec 23, 2024
@Spaarsh
Copy link
Contributor

Spaarsh commented Dec 30, 2024

@zorae the first option seems far better to be since it would immediately make the librarians aware of this new update.

However, I do not feel that any of this should need server-side processing.

If there is a clear consensus about this, then I would like to begin working on this rightaway since it seems simple enough to implement by adding the relevant eventHandlers for edit.html.

@scottbarnes
Copy link
Collaborator

Let's try with this happening upon de-selection of the relevant fields that @zorae enumerated. I went ahead and assigned this to you, @Spaarsh.

@Spaarsh
Copy link
Contributor

Spaarsh commented Dec 31, 2024

@scottbarnes I have added the relevant JS and these are the results:
For editing an existing edition:

edit.page.webm

For adding a new edition:

add.page.webm

For adding a new book:

add.a.new.book.webm

@zorae
Copy link
Author

zorae commented Dec 31, 2024

@Spaarsh looks great! does this handle only normal spaces, or also other whitespace characters like tabs?

@Spaarsh
Copy link
Contributor

Spaarsh commented Dec 31, 2024

@zorae yes! I have used the JavaScript method trim() and the official document defines whitespaces here and it includes tabs as well. It also removes line-terminators such as /n.

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Jan 1, 2025
@scottbarnes scottbarnes removed the Needs: Response Issues which require feedback from lead label Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lead: @scottbarnes Issues overseen by Scott (Community Imports) Priority: 3 Issues that we can consider at our leisure. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed]
Projects
None yet
6 participants