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

MudTabs: add WrapHeaders (reverted) #9108

Merged
merged 6 commits into from
Jun 7, 2024

Conversation

ZephyrZiggurat
Copy link
Contributor

@ZephyrZiggurat ZephyrZiggurat commented Jun 4, 2024

Description

Adds WrapHeaders parameter to MudTabs. If true, tab headers will wrap instead of scroll.

resolves #6758

How Has This Been Tested?

Visually in docs and added WrapHeaders test.

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

image

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

@github-actions github-actions bot added enhancement New feature or request PR: needs review labels Jun 4, 2024
Copy link

codecov bot commented Jun 4, 2024

Codecov Report

Attention: Patch coverage is 89.18919% with 4 lines in your changes missing coverage. Please review.

Project coverage is 90.65%. Comparing base (28bc599) to head (57c30e3).
Report is 256 commits behind head on dev.

Files Patch % Lines
src/MudBlazor/Components/Tabs/MudTabs.razor.cs 89.18% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #9108      +/-   ##
==========================================
+ Coverage   89.82%   90.65%   +0.82%     
==========================================
  Files         412      398      -14     
  Lines       11878    12420     +542     
  Branches     2364     2418      +54     
==========================================
+ Hits        10670    11259     +589     
+ Misses        681      623      -58     
- Partials      527      538      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ScarletKuro
Copy link
Member

It might be a noob question, but @danielchalmers, do you happen to know if a pure CSS solution is possible? Is it really not possible to have a responsive grid without JavaScript for something as simple as an inline header on resize?

@ZephyrZiggurat
Copy link
Contributor Author

It might be a noob question, but @danielchalmers, do you happen to know if a pure CSS solution is possible? Is it really not possible to have a responsive grid without JavaScript for something as simple as an inline header on resize?

If the slider could just be directly inside the tab header element it's possible, but then the slider animation won't function. That's my understanding at least, but I definitely could be wrong.

@danielchalmers
Copy link
Contributor

danielchalmers commented Jun 5, 2024

Btw I tested it and the slider position breaks if you maximize the window.

For pure CSS, are you able to add a flex-wrap: wrap to .mud-tabs-toolbar-content .mud-tabs-toolbar-wrapper and remove width: max-content, then change the way the transform is done?

image

@ZephyrZiggurat
Copy link
Contributor Author

For pure CSS, are you able to add a flex-wrap: wrap to .mud-tabs-toolbar-content .mud-tabs-toolbar-wrapper and remove width: max-content, then change the way the transform is done?

That's exactly what I did to make the headers inline. (I prevent the transform from updating so there's no scrolling.)
Do you have any ideas on a cleaner way to position the slider?

image

Btw I tested it and the slider position breaks if you maximize the window.

If there's no better ideas (see above)... i'll have to play with it some more. The resize events are tricky 🙁

@danielchalmers
Copy link
Contributor

Yeah with the way the slider works JavaScript could make the most sense. I wonder if there's any appetite to change the animation to something more modern (no slider)

@ZephyrZiggurat
Copy link
Contributor Author

(does it ping people when more commits are added?)

The slider is positioning correctly when the window gets resized now.

@henon
Copy link
Collaborator

henon commented Jun 6, 2024

We need a better parameter name. I suggest Wrap="true" instead.

@ZephyrZiggurat
Copy link
Contributor Author

We need a better parameter name. I suggest Wrap="true" instead.

I was worried simply Wrap would be ambiguous. How about TabHeadersWrap or HeadersWrap? I think it should be something that doesn't imply the body might be affected too.

@Anu6is
Copy link
Contributor

Anu6is commented Jun 6, 2024

WrapHeader

@henon
Copy link
Collaborator

henon commented Jun 6, 2024

I think it should be something that doesn't imply the body might be affected too.

Fair enough, WrapHeaders then.

@ZephyrZiggurat ZephyrZiggurat changed the title MudTabs: add TabHeadersInline MudTabs: add WrapHeaders Jun 6, 2024
@henon henon merged commit e5bdc0e into MudBlazor:dev Jun 7, 2024
4 checks passed
@ZephyrZiggurat ZephyrZiggurat deleted the tab-headers-inline branch June 10, 2024 13:54
@henon
Copy link
Collaborator

henon commented Jun 18, 2024

This PR seems to cause a regression: #9167

@henon
Copy link
Collaborator

henon commented Jun 18, 2024

Regression fixed by #9205

henon added a commit to henon/MudBlazor that referenced this pull request Jun 21, 2024
henon added a commit that referenced this pull request Jun 21, 2024
@henon henon changed the title MudTabs: add WrapHeaders MudTabs: add WrapHeaders (reverted) Jun 21, 2024
@henon
Copy link
Collaborator

henon commented Jun 21, 2024

This has been reverted in #9234, the reasons are stated in that PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request PR: needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to make MudTabs that reach the end of the screen to go to the next line.
5 participants