-
Notifications
You must be signed in to change notification settings - Fork 126
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
#453: Add new data validation: ListFormulaDataValidation. #454
Conversation
Add the possibility to set global visibility to range when a named range is added.
@meiMingle @rzymek can you check the pull request and if you think the change makes sense, merge it into master? |
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.
I would be nice if you'd add some tests covering those changes
public void write(Writer w) throws IOException { | ||
w | ||
.append("<dataValidation sqref=\"") | ||
.append(range.toString()) |
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.
unnessesary .toString()
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.
If I try to append the range without "toString", an error is return
*/ | ||
public Range setGlobalVisibility(boolean globalVisibility) { | ||
this.globalVisibility = globalVisibility; | ||
return this; |
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.
other setters return void. returning this here seems inconsistent
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.
I understand. I change it
@@ -44,6 +44,11 @@ public class Range implements Ref { | |||
*/ | |||
private final int right; | |||
|
|||
/** | |||
* Check about the Range visibility when a named range is added. |
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.
I find this comment confusing. Doesn't seem to turn on any "Check", but rather control the scope of this range (right?)
@rzymek thank you for your comments and your time. |
Please @rzymek, can you close the pull request if all is ok? Otherwise can you tell me what needs to be fix to complete the pull request? |
Fixing the issue 453:: a new data validation is added:
ListFormulaDataValidation
.This
DataValidation
allow to create a data validation withlist
type and with aformula
value (to referrer a list, for example withINDIRECT
formula).In addition, it is possibility to set global visibility to a range when a named range is added.