Skip to content
Snippets Groups Projects

Fix date filter input

Merged Petr Stribny requested to merge 2945-fix-date-filter-input into develop
1 unresolved thread

What is in this MR

  • E.g. a short explanation of what this MR does.

How to test this MR

  • E.g. a short series of steps on how to test the features/bug fixes in this MR.

Merge Request Checklist

  • A changelog entry has been created if required.
  • New/updated Premium/Enterprise features are separated correctly in the premium or enterprise folder
  • The latest Chrome and Firefox have been used to test any new frontend features
  • Documentation has been updated
  • Quality Standards are met
  • Performance: tables are still fast with 100k+ rows, 100+ field tables
  • The redoc API pages have been updated for any REST API changes
  • Our custom API docs are updated for changes to endpoints accessed via api tokens
  • The UI/UX has been updated following UI Style Guide

Closes #2945 (closed)

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
182 181 }
183 182
184 183 if (newDate.isValid()) {
185 this.setCopy(newDate.format('YYYY-MM-DD'), sender)
184 const dateString = newDate.format('YYYY-MM-DD')
185 this.setCopy(this.prepareValue(dateString), sender)
  • Author Developer

    For some reason the setCopy method expects the combined string and then parses it again...

  • What if we call this.setValue instead?

    Suggested change
    185 this.setCopy(this.prepareValue(dateString), sender)
    185 this.setValue(dateString, sender)
  • Author Developer

    It does seem to work, replacing both lines. I guess it is a bit weird because the method is called setCopyFromDateString and then we are calling setValue... but I am not gonna think about it more, the whole code seems convoluted to me in this area.

  • I agree the setValue function is weird because is not actually setting the value, but just the copy. What we have to do here is basically:

    this.copy = value
    this.delayedUpdate(value, true)

    That's the reason why I suggested to use setValue, but if it looks a bit less convoluted, we can explicitly set the copy and call the delayedUpdate. Either ways is fine with me

    Edited by Davide Silvestri
  • Author Developer

    No I agree with you that the one line is better, I guess I just don't understand the original reasoning for the data flows here

  • Petr Stribny changed this line in version 2 of the diff

    changed this line in version 2 of the diff

  • Please register or sign in to reply
  • Petr Stribny added 1 commit

    added 1 commit

    Compare with previous version

  • Davide Silvestri approved this merge request

    approved this merge request

  • Petr Stribny enabled an automatic merge when all merge checks for 1300cb85 pass

    enabled an automatic merge when all merge checks for 1300cb85 pass

  • merged

  • Petr Stribny mentioned in commit becc8016

    mentioned in commit becc8016

  • Peter Evans mentioned in merge request !2968 (merged)

    mentioned in merge request !2968 (merged)

  • Please register or sign in to reply
    Loading