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

Improve input validation #182

Open
btzr-io opened this issue Oct 28, 2019 · 10 comments

Comments

@btzr-io
Copy link
Owner

@btzr-io btzr-io commented Oct 28, 2019

The current inputs on the toolbar component requires some improvements:

  • Validation for current page input
  • Validation for current zoom level
@mskuybeda

This comment has been minimized.

Copy link
Contributor

@mskuybeda mskuybeda commented Oct 28, 2019

May I give it a try?

@btzr-io

This comment has been minimized.

Copy link
Owner Author

@btzr-io btzr-io commented Oct 28, 2019

@mskuybeda Sure

@mskuybeda

This comment has been minimized.

Copy link
Contributor

@mskuybeda mskuybeda commented Oct 28, 2019

A small question though, what do you want to validate in Zoom level? And what inputs need to be validated other than preview?

@btzr-io

This comment has been minimized.

Copy link
Owner Author

@btzr-io btzr-io commented Oct 28, 2019

@mskuybeda Right now you can't remove all numbers while typing so its annoying ( there is always a number displayed cant be an empty string ) and is hard to use.

Also we should check for min and max values, currenty the min value in zoom is a dynamic value generated inside the render component, and the max value is "100%",

@btzr-io

This comment has been minimized.

Copy link
Owner Author

@btzr-io btzr-io commented Oct 28, 2019

Check the currentPage input on the naviagtion component.

@mskuybeda

This comment has been minimized.

Copy link
Contributor

@mskuybeda mskuybeda commented Oct 28, 2019

So for zoom, do you want to make a minimum as low as 0% for all images? Or enable user to change it to any value?

@btzr-io

This comment has been minimized.

Copy link
Owner Author

@btzr-io btzr-io commented Oct 28, 2019

No the min zoom value is calculated on the render component

@mskuybeda

This comment has been minimized.

Copy link
Contributor

@mskuybeda mskuybeda commented Oct 28, 2019

Ok. Still not really sure what do you want to change to a current state. Right now as I understand it calculates min zoom value depending on the image. Different images have different min value. What would you like to change about it?

Secondly, I have almost finished preview. Would you like it to appear empty on load or to still have now erasable 0

@btzr-io

This comment has been minimized.

Copy link
Owner Author

@btzr-io btzr-io commented Oct 28, 2019

Empty is fine, thanks ✌️

@mskuybeda

This comment has been minimized.

Copy link
Contributor

@mskuybeda mskuybeda commented Oct 29, 2019

Created a pull request for preview line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.