department-of-veterans-affairs / vets-design-system-documentation

Repository for design.va.gov website
https://design.va.gov
37 stars 57 forks source link

va-file-input #2886

Open rmessina1010 opened 3 months ago

rmessina1010 commented 3 months ago

Description

I. This is proposing to remove, or change the handling of, the error attribute va-file-input. At the time of writing, the error attribute is a string that is set when defining a va-file input component; this is then serves double as BOTH the message displayed and flag to the error state of said component. The fact that error is an attribute would allow a VFS team to create , intentionally or not, va-file-input that is stuck on error mode e.g. : <VaFileInput error="Am the error message ... but also the existence of this string set at creation just determined this element to have an error" hint="You can upload a .pdf, .gif, .jpg, .bmp, or .txt file." name="my-file-input" uswds />

I could be wrong, maybe error was meant to be used not as an attribute but more along the line of a property to be set externally, inside a validation function e.g.: const vaFileInputNode = document.querySelector('va-file-input); vaFileInputNode.error = "oopsies"; at the moment I am just failing to see how an external validation funtion would get the file data it needs to validate the file attached inside the va-file-input component. Which leads to...

II. Evaluate vaCange(). As per our discussions this would be the hook VFS teams would use to PASS the logic to determine whether a the input has an error or not and then set the message. a) This leads me back to question that if this is the case ( where a passed function determines the error message value) but the VFS enginner can also set the declare the value of the prop when hard coding the element ... it can lead to confusion as which is the actual source of truth for the state of the element? b) Secondly, because every other change is also handled by what is passed as a function via vaChange it leads to possible conflicts of interest. Tho am building a workaround for , here is an a scenario: right now: multiple-file-input passes a function via vaChange ... if onchange is triggered on the input... the parent va-file-input is taken out of the DOM and added to array of inputs in the state of multiple-file-inputs but this function it taking the spot whee file validation would go.

private handleChange(e) {
    console.log('just changed');
    if (e.target.id === this.fileTrackCount.toString()) {
      this.registerFileInput();
    }
  }

  private registerFileInput(): void {
    console.log(this.inputOnDeck, 'reg');
    this.inputArray.push(this.inputOnDeck);
    this.inputOnDeck = this.makeNewInput();
    return;
  }
   ...
private createInput(): HTMLElement {
    const {
      name,
      required,
      accept,
      error,
      // uploadPercentage,
      headerSize,
      enableAnalytics,
    } = this;
    const inputElement = document.createElement('va-file-input');
    inputElement.id = this.fileTrackCount.toString();
    inputElement.name = name;
    inputElement.accept = accept;
    inputElement.required = required;
    inputElement.error = error;
    inputElement.uswds = true;
    inputElement.headerSize = headerSize;
    inputElement['enable-analytics'] = enableAnalytics;
    inputElement.addEventListener('vaChange', e => this.handleChange(e));
    inputElement.addEventListener('vaRemoveFile', e =>
      this.handleRemoveFile(e),
    );
    inputElement.setAttribute('ischild', 'true');
    return inputElement; 
  }

Even this example is a bit odd, as I had to use addEventListener() to add my function, but most teams would compose it using <VaFileInput ... vaChange = { someFoo()} />

Tasks

Acceptance Criteria

caw310 commented 3 months ago

Hey team! Please add your planning poker estimate with Zenhub @Andrew565 @ataker @harshil1793 @it-harrison @jamigibbs @micahchiang @nickjg231 @powellkerry @rmessina1010 @rsmithadhoc

jamigibbs commented 3 months ago

@rmessina1010 Can you please give this issue a more descriptive name instead of just "va-file-input"?