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

Rename WithoutLabel to WithoutLabelElement and deprecate it #122

Closed
zabulus opened this issue May 5, 2015 · 4 comments · Fixed by #123
Closed

Rename WithoutLabel to WithoutLabelElement and deprecate it #122

zabulus opened this issue May 5, 2015 · 4 comments · Fixed by #123

Comments

@zabulus
Copy link
Contributor

zabulus commented May 5, 2015

Here is a test in DefaultFieldGeneratorTest that fails:

        [Test]
        public void GetLabelHtml_should_not_return_any_string_if_WithoutLabel_used_and_DisplayAttribute_present()
        {
            var generator = Arrange(x => x.StringWithDisplayAttribute);
            var fieldConfig = new FieldConfiguration();
            fieldConfig.WithoutLabel();
            var config = generator.PrepareFieldConfiguration(fieldConfig, FieldParent.Section);
            var actual = generator.GetLabelHtml(config).ToString();
            Assert.That(actual, Is.Empty);
        }
zabulus pushed a commit to zabulus/ChameleonForms that referenced this issue May 5, 2015
GetLabelHtml returns not empty string if Display Attribute present
zabulus pushed a commit to zabulus/ChameleonForms that referenced this issue May 5, 2015
GetLabelHtml returns not empty string if Display Attribute present
@robdmoore
Copy link
Member

I think this is a misunderstanding maybe? .WithoutLabel means don't use the label element rather than don't show a label at all?

@zabulus
Copy link
Contributor Author

zabulus commented May 5, 2015

Hm that makes sense. Maybe it should be named WithoutLabelTag? Or maybe add clarification to the documentation.
BTW I thought field configuration should specify what generate and not how generate.

@robdmoore
Copy link
Member

I agree that WithoutLabel is misleading. The xmldoc should make it clear what it does.

I'm hesitant to make a breaking rename change because this code is called from razor views which most people don't compile. We could create an alias name that is easier to understand though (but simply calls WithoutLabel). We could even mark WithoutLabel obsolete so people don't use it from now on?

As to what the method should be called. What about WithoutLabelElement?

zabulus pushed a commit to zabulus/ChameleonForms that referenced this issue Sep 7, 2015
GetLabelHtml returns not empty string if Display Attribute present
@zabulus
Copy link
Contributor Author

zabulus commented Sep 7, 2015

Fix according to last comment

@zabulus zabulus changed the title GetLabelHtml returns not empty string if Display Attribute present Rename WithoutLabel to WithoutLabelElement and deprecate it Sep 7, 2015
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

Successfully merging a pull request may close this issue.

2 participants