Blazor default templates have nav render issues

dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.

https://asp.net

MIT License

35.32k stars 9.97k forks source link

Blazor default templates have nav render issues #7206

Closed robertmclaws closed 1 year ago

robertmclaws commented 5 years ago

The default templates for Blazor have several issues with the CSS that breaks rendering in the following situations:

when the Navigation list contains more than approximately 12 items on either Desktop or Mobile
the browser is full screen on a 4K monitor

In these cases, you will experience one or more of the following issues:

The background gradient will cut off at the end of the list, leaving a different color background in the rest of the column.
At all resolutions, the nav will not scroll properly if there are more items than there is vertical height.
If the text any nav item is too long, it will overlap the items around it.

My company has fixed these issues in our apps, and my colleague will be making a pull request to submit our changes so that the community can leverage the improved rendering in their apps.

mkArtakMSFT commented 5 years ago

Thanks for bringing this up, @robertmclaws. Sure, we would happily consider your PRs.

mkArtakMSFT commented 5 years ago

@SteveSandersonMS, assigned to you to review the PR, when it'll be available.

adrianwright109 commented 5 years ago

@robertmclaws has the PR been submitted yet?

robertmclaws commented 5 years ago

My bad, I got sidetracked by another project. Will attempt to push one this evening. Thanks for the reminder!

peterblazejewicz commented 4 years ago

@robertmclaws Robert, can you attach the screenshots if that is still a problem? I could have a look into this, Thx!

chrissainty commented 4 years ago

I'll have a go at this one if that's ok @SteveSandersonMS? Looks like a bit of CSS needed. I can't find the templates in the repo, could you point me in the right direction?

SteveSandersonMS commented 4 years ago

Awesome, thanks! I think the two you want are:

Running the templates from these directories is difficult, so what I'd recommend is simply figure out what .css contents you want using a separate normal project, then just copy in the resulting .css file here.

Final point, we'd probably only want to take reasonably simple CSS changes. If a fix would require something super complicated, or especially if it requires extra boilerplate HTML, then we may prefer not to change it. That's because the templates serve their main purpose in helping people get started and make sense of how things work.

chrissainty commented 4 years ago

For the menu scrolling issue there are two options I think.

Option 1 is to add overflow-y: auto to the .sidebar class. This will allow it scroll independently of the main content.
Options 2 would be to adjust the .sidebar class's height property to min-height: 100vh;, this will allow it to grow with content and the whole page will scoll as one.

Personally, I think option 2 would be the nicer option.

For the nav text issue this could be solved by updating .sidebar .nav-item a to have a min-height: 3rem instead of just height and removing the width property from .sidebar .oi and replacing it with margin-right: .8rem. The only down side is that if the link text goes multline then the icon will always align to the middle of the height of the text due to the align-items: center rule. Would we want to icon to be aligned to the top of the text? If so I think it could require some HTML changes, potentially.

robertmclaws commented 4 years ago

I don't think scrolling the whole page is the right approach under this design. I personally think the sidebar should be 100vh and let content inside be scrollable as necessary.

I also think that the icon should be centered because it doesn't throw off the balance between the other lines. Example:

Just my $0.02.

TanayParikh commented 2 years ago

Scrolling functionality has been added via https://github.com/dotnet/aspnetcore/pull/41062.

Is anyone still having issues with high resolution monitors and the gradient being cut off?

ghost commented 2 years ago

Hi @robertmclaws. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

ghost commented 2 years ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.

robertmclaws commented 2 years ago

I'll check it out in the next release. If it's fixed and everyone is ok with it, I'm fine closing out the issue.