[HelperText] Purpose of both `validation` and `isValidationMessage`

[dawsonc623]

I was reviewing the code related to validation messages with HelperText. It seems that when the setting the validation attribute, the React component adds the mdc-text-field-helper-text--validation-msg class to the list of classes added to the p created and rendered, while setting isValidationMessage triggers a process telling the foundation object it is for validation which in turn adds the mdc-text-field-helper-text--validation-msg class to a classList state that does not seem to be used at all.

Is there a reason isValidationMessage (or really, classList in general) exists? If not, the code and documentation should be updated to avoid confusion. This could be by getting rid of the local classes method and updating persistent to use the foundation as the other attributes do, or by gutting the use of the foundation object and keeping everything local. I am in favor of the former option since that keeps all of the class management logic in a place consistent with other wrapping frameworks (assuming they also use the foundation classes). I am happy to open a PR with whichever route is chosen.

[moog16]

RE classList: This is actually our pattern thorughout all our components. Our thought process was when we add a class, it'll update state and thus rerender the component. We have been experimenting with just using the DOM's classList, and it seems to work just fine.

RE persistent: What do you mean "use the foundation"? Can you explain further about gutting the foundation? Since the foundation is an important part of the architecture of MDC Web, we use it as much as possible. So removing the foundation altogether isn't really an option for us.

RE PR: Ya I'm all for PRs, we are updating to the latest MDC Web v1.0 to use their Typings. I'm assuming you're familiar with TS based on your other comments (I think that was you). The helper text should be a quick update if you decide to do a PR. Please see tabscroller to get a better idea.

Thanks!

[dawsonc623]

I do not think I communicated my point very clearly. This is what I see in the code (and correct me if I missed something - I am still learning the MDC architecture).

This line pulls the classes from this.classes. this.classes calculates the list of classes using props on the component instead of classList. This means all of the work MDCTextFieldHelperTextFoundation is doing to keep classList up-to-date is moot.

Coming across both validation and isValidationMessage in the documentation confused me, and my confusion deepened when I noticed isValidationMessage did not seem to be doing anything at all. That is why I dug into this.

My note about persisent is simply that the React component is not using foundation_.setPersistent anywhere, but that is not a "problem" because the component is not using classList anyway. My overall point is that this component does not seem to be fully using MDCTextFieldHelperTextFoundation or reaping the benefits it intends to provide, and so it should be updated to use it fully (or not use it at all, but it sounds like that is not the route we want).

[moog16]

@dawsonc623 yes you're totally right! classes should be using this.state.classList, which is a bug. I believe the reason for leaving foundation.setValidation out is because MDC Web's helper text only adds/removes classes. However MDC React should be using all the foundation methods and implementing all the adapter methods. If we don't, there should be a good reason. We will be updating text-field to MDC Web v1.0.1 in the coming weeks. We should keep this bug open and make sure we build helper-text correctly this time.

[moog16]

Also RE isValidationMessage & validation: I think we might also have a bug open for this, and I will remove one or both of them to make this more clear.

[dawsonc623]

Awesome! Thank you for taking a look, and if I can help in anyway let me know. I am more than happy to contribute in any capacity I can.

[moog16]

Thanks for the offer. I will definitely let you know. We are currently waiting to convert the dependencies (line-ripple, notched-outline, floating-label) before we can actually dive into text-field and the auxiliary components. I will follow up once we're there.