-
Notifications
You must be signed in to change notification settings - Fork 4
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
Uri Value Component #22
Conversation
…: don't do that. Also added the rest of the tests for the UriValueComponent.
Thanks, I try to have a look at that today. |
I think the validator could use some work but I'm not entirely sure what Knora accepts as a valid url |
any valid I think it would be a good idea to put the validators in a central place so they could be reused. Also then the regex would only be created once and not with every instance of a value component. |
I'm a little lost on how I can use this. Do we have validation functions set up already for uri's or will we still need to use regex?
I can work on this. It would probably be good to implement it before we create all of the other components. Should I make a new issue for this? |
|
||
it('should set a new display value', () => { | ||
|
||
const newInt = new ReadUriValue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you adapt the variable name? Just for reasons of clarity, doesn't make any technical difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in b228083
}); | ||
}); | ||
|
||
describe('create an integer value', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please adapt the test's name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in b228083
…the URI tests clearer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
closes #17