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

Discrepancy in allowed and displayed temporary file sizes. #527

Closed
guystreeter opened this issue Oct 29, 2021 · 3 comments
Closed

Discrepancy in allowed and displayed temporary file sizes. #527

guystreeter opened this issue Oct 29, 2021 · 3 comments

Comments

@guystreeter
Copy link
Contributor

Hey, Thanks for reporting issues back to Nextcloud Face Recognition. Please, try to complete this report in detail so we can help you easier. 😄

Make sure you read all the documentation, and the FAQ, and that the issue has not been reported before. 😉

Expected behaviour

I should be able to set the file size slider to the maximum allowed by the model.

Actual behaviour

The UI will tell me I can't set it higher than the value I'm trying to set it to.

Steps to reproduce

  1. Using model 4, try to set the slider to 1672x1254
  2. The UI will tel you that you cannot set it higher than 1672x1254

Server configuration

Latest nextcloud apache docker image, with self-built pdlib, and current git clone of facerecognition

Additional information

The problem appears to be the rounding that happens to create the 4x3 values. The slider can be set to something that displays 1672x1254 but is fractionally higher, because the display rounds down.

I think a reasonable fix is to keep the displayed maximum value in the error message rounded down as it is now, but round up on the slider's displayed value.

@guystreeter
Copy link
Contributor Author

Another, possibly better, solution: when the user clicks the save button, convert the slider value to rounded-down width and height, then multiply those get a value to be set.

@guystreeter
Copy link
Contributor Author

A third option: set the max value of the slider to the maximum allowed by the model, since you don't let the user set it any higher than that anyway. I've never tried to write javascript, so I have no idea how to implement that.

guystreeter added a commit to guystreeter/facerecognition that referenced this issue Oct 31, 2021
(if the model has been set). Saving a value higher than that is disallowed
anyway, and this resolves the promlem reported in matiasdelellis#527
@matiasdelellis
Copy link
Owner

Thank again @guystreeter 🎉 😄

I couldn't test it, but it was 100% logical.. 😬
When you want, close this report yourself. 😉

p.s: Quick answer... I hope to have free time soon to do a few things before NC23

matiasdelellis added a commit that referenced this issue Nov 20, 2021
- Initial support for php 8. See issue #456
- Add link to show photos of person on sidebar.
- Add static analysis, phpunit and lintian test using github workflow.
- Add an real OCS public API to get all persons. See PR  #512.

- Fix sidebar view when user has disable it.
- Set the Image Area slider to the maximum allowed by the model. See issue #527
- Don't try to force the setCreationTime argument to be DateTime. See PR #526
- Migrate hooks to OCP event listeners. See PR #511

- New Czech translation thanks to Pavel Borecki, and update others. Thank you so
  much everyone.
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

No branches or pull requests

2 participants