carbon-design-system / carbon-components-angular

An Angular implementation of the Carbon Design System for IBM.
https://angular.carbondesignsystem.com
Apache License 2.0
532 stars 307 forks source link

DatePicker and DatePickerInput implementation duplicates too many classes #2822

Open klaascuvelier opened 7 months ago

klaascuvelier commented 7 months ago

Describe in detail the issue you're having.

I noticed that the implementation for datepicker.component.ts and datepicker-input.component.ts have quite some duplication in the template.

This results in the DOM with a structure like this

<!-- start datepicker.component.ts -->
<div class="cds--form-item">
  <div class="cds--date-picker">
    <div class="cds--date-picker-container">
      <!-- start datepicker-input.component.ts -->
      <div class="cds--form-item">
        <div class="cds--date-picker">
          <div class="cds--date-picker-container">
            <label>...</label>
            <div class="cds--date-picker-input__wrapper>
              <span>
                <input />
              </span>
            </div>
          </div>
        </div>   
      </div>
      <!-- end datepicker-input.component.ts -->
    </div>
  </div>
</div>
<!-- end datepicker.component.ts -->

This is a pretty bloated DOM structure and can be simplified a lot. The react implementation is a lot simpler and has not class duplication.

I created this table as comparison, hope this works to show the problem:

component angular react
datepicker .cds--form-item > .cds--date-picker > .cds--date-picker--container .cds--form-item > .cds--date-picker
datepicker-input .cds--form-item > .cds--date-picker > .cds--date-picker--container .cds--date-picker--container
klaascuvelier commented 7 months ago

I just found out possible 2 reasons why it might be implemented this way:

Akshat55 commented 7 months ago

I agree with you on this! The structure is bloated and the change is a breaking change. I'm thinking it might make sense to make the PR target the next branch where we can release a rc version and release it officially with v12 next year. 🤔

klaascuvelier commented 7 months ago

Hi @Akshat55 thanks for the feedback. I agree that maybe targetting a later release is fine. For us this is not a blocking issue, just a nice improvement :) Let me know if there's something more I can do.