-
Notifications
You must be signed in to change notification settings - Fork 1
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
BI-674 - Program Definable BrAPI Service URL #75
Conversation
1b0d9b5
to
2b5e1db
Compare
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!
rather than the verbiage next to the checkbox "Specify custom program data storage location", in order to satisfy the first acceptance scenario, would it be beneficial for the text to read "Specify custom program data storage location, otherwise default will be used"? |
I see what you're saying with the description but to me it reads more cleanly to have the checkbox label be more yes/no. After talking with Tim I updated the acceptance criteria to not specify what the default location is because there was implementation implications that didn't seem worth it. Not sure how others feel? Updated text to say "system default" to match how bi-api is now returning "System Default" rather than the docker URL. |
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.
See comment on UI above.
I wrapped the input in a label and in browser that added some more space, does it look good on yours?
I added an example url to the help text, seems the validation message is automatically generated. I just hardcoded an example for url validation, could be improved in future. |
Changes
Notes