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

Support hidden label in fieldset #7583

Merged
merged 9 commits into from Aug 11, 2023

Conversation

josefbehr
Copy link
Contributor

  • Changes have been thoroughly tested to not break existing functionality.
  • New functionality has been documented or existing documentation has been updated to reflect changes.
  • Visual changes are explained in the PR description using a screenshot/recording of before and after.

before:
image

after:
image

@what-the-diff
Copy link
Contributor

what-the-diff bot commented Aug 9, 2023

PR Summary

  • Enhanced User Interface Controls
    The implementation of the :label-hidden="$isLabelHidden()" attribute in the fieldset.blade.php file across several directories - namely packages/forms, packages/infolists, and packages/support - improves the usability of our web elements. This change makes it possible to hide and show labels dynamically, providing greater control over the user interface experience.

@josefbehr
Copy link
Contributor Author

I know that the label can be hidden by just setting an empty string as the label, but I think it should respect the hiddenLabel() method as well, which also enables dynamically showing or hiding the label.

@zepfietje
Copy link
Member

Thanks for the PR, @josefbehr.
I've updated the implementation.
However, I think we should potentially still output the label, but add sr-only.

@zepfietje zepfietje added the bug Something isn't working label Aug 9, 2023
@zepfietje zepfietje added this to the v3 milestone Aug 9, 2023
@danharrin
Copy link
Member

danharrin commented Aug 9, 2023

We should add a label-hidden attribute to the fieldset Blade component that handles sr-only

@zepfietje
Copy link
Member

zepfietje commented Aug 10, 2023

We should add a label-hidden attribute to the fieldset Blade component that handles sr-only

Agree. Mark as ready for review when you've handled that, @josefbehr. 🙂

@zepfietje zepfietje marked this pull request as draft August 10, 2023 07:05
@zepfietje zepfietje changed the title hide fieldset label if hiddenLabel() is called Support hidden label in fieldset Aug 10, 2023
@josefbehr
Copy link
Contributor Author

done

@josefbehr josefbehr marked this pull request as ready for review August 11, 2023 05:35
@danharrin danharrin merged commit bd32089 into filamentphp:3.x Aug 11, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants