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

Writer toolbar: hide when no buttons #6846

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

distantnative
Copy link
Member

@distantnative distantnative commented Dec 7, 2024

Changelog

Fixes

Ready?

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and CI checks all pass

For review team

  • Add lab and/or sandbox examples (wherever helpful)
  • Add changes & docs to release notes draft in Notion

@distantnative distantnative added this to the 4.6.0 milestone Dec 7, 2024
@distantnative distantnative requested a review from a team December 7, 2024 18:22
@distantnative distantnative self-assigned this Dec 7, 2024
@distantnative distantnative marked this pull request as ready for review December 7, 2024 18:27
Copy link
Member

@afbora afbora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the target branch be develop-minor for 4.6.0?

@distantnative
Copy link
Member Author

@afbora not necessarily. We put everything that could be released in a patch release on develop-patch - in that case that we would want to release a 4.5.1, we could do that right away with the necessary changes already on develop-patch. Since we continuously merge develop-patch into develop-minor, if we decide against any patch release, they're automatically included in the next minor release.

@bastianallgeier
Copy link
Member

I'm totally fine to include it in the patch release, but I think we should remove the :has change in that case, right?

@distantnative
Copy link
Member Author

distantnative commented Dec 12, 2024

You're right, we only started adding :has() back with v5.... mm but without that change it's still broken/needs additional changes in other components.

@distantnative
Copy link
Member Author

@bastianallgeier I'd suggest then to move it to v5. I think it's rather complicated to fix this without :has() as the style is applied to .k-writer not the toolbar itself. I though about introducing another data attribute, but then we would have to duplicate the whole buttons logic from k-writer-toolbar in k-writer to make it consistent.

@bastianallgeier bastianallgeier modified the milestones: 4.6.0, 5.0.0, 5.0.0-beta.2 Dec 16, 2024
@bastianallgeier bastianallgeier changed the base branch from develop-patch to v5/develop December 16, 2024 11:26
@bastianallgeier
Copy link
Member

@distantnative can you fix the conflicts?

@distantnative distantnative force-pushed the fix/6804-writer-toolbar-hide-empty branch from 4b810c1 to 33138ee Compare December 17, 2024 08:48
@distantnative distantnative force-pushed the fix/6804-writer-toolbar-hide-empty branch from 33138ee to fb4a468 Compare December 17, 2024 08:54
@distantnative
Copy link
Member Author

@bastianallgeier done

@bastianallgeier bastianallgeier merged commit afa1532 into v5/develop Dec 17, 2024
13 checks passed
@bastianallgeier bastianallgeier deleted the fix/6804-writer-toolbar-hide-empty branch December 17, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Writer Fields Non-inline toolbar should not show when no marks or nodes are available
3 participants