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

Short Name empty #200

Closed
goaround opened this issue Jul 28, 2019 · 5 comments · Fixed by #201
Closed

Short Name empty #200

goaround opened this issue Jul 28, 2019 · 5 comments · Fixed by #201
Milestone

Comments

@goaround
Copy link

I have a problem with the Regex used here to shorten the page name: https://github.com/xwp/pwa-wp/blob/8fbaf8b928f60aaf7477e2fdae7e19baae55f0cf/wp-includes/class-wp-web-app-manifest.php#L139

My domain name is exacly 12 characters long, following by a dot TLD: "Travel-Dealz.de". "Travel-Dealz" would be a beautiful shortname but because the Regex /^.{0,12}(?= |$)/ only looks for spaces, my short_name is completly emtpy: https://regexr.com/4i7mt

My proposal is to look for spaces and dots: /^.{0,12}(?=[ .]|$)/ see here https://regexr.com/4i7mq

@westonruter
Copy link
Collaborator

Yeah, I think we should just eliminate trying to automatically derive a short_name. It should be supplied via filter instead. Please take a look at #181 and see if you agree.

@goaround
Copy link
Author

Is there really a limit of 12 chars? I use the filter too and think its better to eliminate the regex.

@westonruter
Copy link
Collaborator

There is no hard limit on 12 characters, as far as I know. The limit is just to be short enough to fit with an icon.

Ref. https://developer.mozilla.org/en-US/docs/Web/Manifest/short_name

@westonruter
Copy link
Collaborator

Actually, Chrome recommends a 12 character limit for short_name: https://developer.chrome.com/apps/manifest/name#short_name

But it isn't reliable to compute this automatically, so let's get rid of it. It should only be supplied by a site explicitly.

@westonruter
Copy link
Collaborator

I've opened a PR to remove it: #201

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 a pull request may close this issue.

2 participants