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

Add names characters limit and truncate #1754

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

nhanaa
Copy link

@nhanaa nhanaa commented Mar 16, 2024

Describe your changes

  • Added truncate string util function to truncate string if over certain length
  • Added limits to names and placeholders to indicate the limits (300 for team and loadout, 100 for build)
  • Truncate the names displayed (300 for team and loadout, 50 for build)

Issue or discord link

Testing/validation

image
image
image
image

Checklist before requesting a review (leave this PR as draft if any part of this list is not done.)

  • I have commented my code in hard-to understand areas.
  • I have made corresponding changes to README or wiki.
  • For front-end changes, I have updated the corresponding English translations.
  • I have run yarn run mini-ci locally to validate format and lint.
  • If I have added a new library or app, I have updated the deployment scripts to ignore changes as needed

@nhanaa nhanaa changed the title Add names char limit Add names characters limit Mar 16, 2024
@nhanaa nhanaa changed the title Add names characters limit Add names characters limit and truncate Mar 16, 2024
@nhanaa nhanaa marked this pull request as ready for review March 16, 2024 06:02
@nhanaa nhanaa marked this pull request as draft March 16, 2024 06:03
@nhanaa nhanaa marked this pull request as ready for review March 16, 2024 06:03
Copy link
Contributor

github-actions bot commented Mar 16, 2024

[sr-frontend] [Sat Mar 16 06:43:18 UTC 2024] - Deployed 81e72ee to https://genshin-optimizer-prs.github.io/pr/1754/sr-frontend (Takes 3-5 minutes after this completes to be available)

[frontend] [Sat Mar 16 06:44:17 UTC 2024] - Deployed 81e72ee to https://genshin-optimizer-prs.github.io/pr/1754/frontend (Takes 3-5 minutes after this completes to be available)

[sr-frontend] [Sat Mar 16 11:23:03 UTC 2024] - Deployed c2df5de to https://genshin-optimizer-prs.github.io/pr/1754/sr-frontend (Takes 3-5 minutes after this completes to be available)

[frontend] [Sat Mar 16 11:24:07 UTC 2024] - Deployed c2df5de to https://genshin-optimizer-prs.github.io/pr/1754/frontend (Takes 3-5 minutes after this completes to be available)

Copy link
Owner

@frzyc frzyc left a comment

Choose a reason for hiding this comment

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

I think more truncation may be needed for team and loadout names:

In format of ( display / max string length in db)

  • team ( 100 / 200)
  • loadout ( 100 / 200)
  • Build (50 /100)

For truncating the string(enforce final length), I'd recommend doing it in the database.
Since all data is validated in the database, ensuring string length should be as well:
code that validates loadout(TeamChar) name string


// truncate string to a certain length, and add ellipsis if it's longer
export function truncateString(str: string, length: number) {
return str.length > length ? str.slice(0, length) + '...' : str
Copy link
Owner

@frzyc frzyc Mar 16, 2024

Choose a reason for hiding this comment

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

Suggested change
return str.length > length ? str.slice(0, length) + '...' : str
return str.length > length ? str.slice(0, length - 3) + '...' : str

@nhanaa
Copy link
Author

nhanaa commented Mar 17, 2024

I think more truncation may be needed for team and loadout names:

In format of ( display / truncate)

  • team ( 100 / 200)
  • loadout ( 100 / 200)
  • Build (50 /100)

For truncating the string(enforce final length), I'd recommend doing it in the database. Since all data is validated in the database, ensuring string length should be as well: code that validates loadout(TeamChar) name string

By doing it in the database, do you mean truncating the string before saving the names in the database?

@frzyc
Copy link
Owner

frzyc commented Mar 17, 2024

I think more truncation may be needed for team and loadout names:
In format of ( display / max string length)

  • team ( 100 / 200)
  • loadout ( 100 / 200)
  • Build (50 /100)

For truncating the string(enforce final length), I'd recommend doing it in the database. Since all data is validated in the database, ensuring string length should be as well: code that validates loadout(TeamChar) name string

By doing it in the database, do you mean truncating the string before saving the names in the database?

I guess I should be more specific since your function is called truncateString. Slicing the string down to max size, should happen on the database size, before the name is saved. Applying the ellipsis, should be done on the UI(and does not change the database string size).

@frzyc frzyc marked this pull request as draft April 3, 2024 02:45
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.

None yet

2 participants