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

Fix wire:dirty.remove not restoring element once it is no longer dirty #6759

Merged
merged 4 commits into from
Sep 13, 2023

Conversation

PhiloNL
Copy link
Collaborator

@PhiloNL PhiloNL commented Sep 11, 2023

Failing test for #6668.

new class extends Component {
  public $prop = false;
  
  public function render()
  {
      return <<<'BLADE'
          <div>
              <input dusk="checkbox" type="checkbox" wire:model="prop" value="true"  />
  
              <div wire:dirty>Unsaved changes...</div>
              <div wire:dirty.remove>The data is in-sync...</div>
          </div>
      BLADE;
  }
}

The data is in-sync... is shown on the initial load. When you check the checkbox, the message disappears but if you uncheck the checkbox the The data is in-sync... will not show again even though it's back in its original state. The same behavior occurs when using text inputs so It's not related to just checkboxes.

@joshhanley
Copy link
Member

@PhiloNL thanks for the failing test!

The issue is the shared function toggleBooleanStateDirective caches and uses the current display value (if there is one) for resetting the element that has the .remove modifier on it.

This works great for wire:loading.remove as after the request is finished and the element has been morphed, the element no longer has the display: none set on it.

But in the scenario of wire:dirty.remove the display value is set to display:none by wire:dirty when it initially removes the element after a dirty has been detected. Once dirty is no longer true, it reverts the elements display value back to its "original" state, which is being calculated as display:none as that is what is currently on the element.

So what we need to do is store the display value of the element when the directive is initialised, so it can be reverted back to that value.

@calebporzio I've added the option to pass the element's initial display value to the toggleBooleanStateDirective function. I'm not sure about that API though, if you have any other suggestions.

Hope this helps!

# Conflicts:
#	dist/livewire.min.js
#	dist/manifest.json
@joshhanley joshhanley changed the title Inconsistent behavior when using wire:dirty.remove Fix wire:dirty.remove not restoring element after it no longer being dirty Sep 12, 2023
@joshhanley joshhanley changed the title Fix wire:dirty.remove not restoring element after it no longer being dirty Fix wire:dirty.remove not restoring element once it is no longer dirty Sep 12, 2023
@calebporzio
Copy link
Collaborator

Dig it, thanks Josh!

@calebporzio calebporzio merged commit 877d5c1 into livewire:main Sep 13, 2023
0 of 2 checks passed
@lucasromanojf
Copy link
Contributor

Hope this fix works for wire:loading.remove too. It has the same problem. Gonna test it next week

@joshhanley
Copy link
Member

@lucasromanojf oh really? I'll do a quick test. Have just tagged a release v3.0.4 with this fix.

@joshhanley
Copy link
Member

joshhanley commented Sep 14, 2023

@lucasromanojf seems like wire:loading.remove is working, but it's not using the default display value, instead it's using inline-block. I'll see if I can get that fixed

Edit: actually I was wrong, it all seems to be working happily to me. @lucasromanojf can you do a test and let me know? Thanks!

@lucasromanojf
Copy link
Contributor

But it is good to hear it is working, even with inline-block. This fix may have fixed it too. Last week I was doing some tests with a button labeled "Save" which should change to "Please wait...". "Please wait" was shown and "Save" was hidden (correct), but after loading finished, the button was empty ("please wait" was hidden correctly but "Save" was not shown)

@lucasromanojf
Copy link
Contributor

@lucasromanojf seems like wire:loading.remove is working, but it's not using the default display value, instead it's using inline-block. I'll see if I can get that fixed

Edit: actually I was wrong, it all seems to be working happily to me. @lucasromanojf can you do a test and let me know? Thanks!

Sure, I'm on a trip but I will test it next week. Thanks!!!

@joshhanley
Copy link
Member

@lucasromanojf ah interesting! Yeah hopefully all sorted now. No worries, enjoy your trip! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants