Geta / geta-notfoundhandler

The popular NotFound handler for ASP.NET Core and Optimizely, enabling better control over your 404 page in addition to allowing redirects for old URLs that no longer works.
Apache License 2.0
19 stars 15 forks source link

Admin screens do not overflow with Opti 12.22.0 #95

Open andrewmarkhamUNRVLD opened 11 months ago

andrewmarkhamUNRVLD commented 11 months ago

Hi,

With the latest release of Optimizely (12.22.0), the Admin screens are no longer responsive so the bottom of the page is cut off and the pagination isn't visible.

image

As you can see on the screenshot above the scroll bar is cut off.

The fix is very simple, for the containing div

overflow:auto height: calc(100vh - 40px)

GeekInTheNorth commented 11 months ago

Are you waiting to see if Optimizely fix the overflow on their menu changes themselves or will you be releasing a fix for this? I could raise a PR if required.

GeekInTheNorth commented 11 months ago

@marisks Are you able to review this?

marisks commented 11 months ago

Can you make a PR? P.S. Optimizely, broke UI, and now it sucks even with a fix it is strange - double left menus. As NotFoundHandler is not just for Optimizely, its menu makes sense in an ASP.NET app, but in Optimizely it seems wrong.

andrewmarkhamUNRVLD commented 11 months ago

I have raised a support ticket with Opti around them breaking the interface, but they don't consider it an issue their end, and it's for 3rd parties to fix. I am not sure that I agree with this sentiment, but TBH I didn't have the energy to get into an argument.

I am happy to make a hotfix and PR for this, as I would like to get a fix out. Give me a few days

andrewmarkhamUNRVLD commented 11 months ago

Maybe something to bring up in the slack optimizely slack channel

marisks commented 11 months ago

That's ok if they change UI. But then they need documentation on how to integrate their UI with 3rd party UIs. Our current solution is also just found by trial and error. Maybe they have documentation now though, I haven't checked it lately.

GeekInTheNorth commented 11 months ago

Maybe an interim fix for now?

I do get where you are coming from. I changed the menus on another plug-in because of top top top menus so having this issue of left left menus is equally frustrating.

I do wish they had better documentation for plugging into their menus and personally I do consider this a breaking change by Optimizely, which seems to be a bit of pattern right now.

I'm happy to add my support to whatever is raised in the slack channels.

DeepRed commented 11 months ago

Just wanted to comment on your discussion here. We changed the menu system in adapting to a common UI to make all Optimizely products look and behave more the same. Our goal is always not to introduce breaking changes. I personally tried many new addons with the menu system and we fixed many issues. However, Not found handler was tested but not with enough data to require the scroll, and that was in retrospect something we should have done. Geta tags, Sql Studio and others were also among the list that were tried.

Yesterday I ran tests with Not found handler and could replicate the scrolling issue. We have registered a bug for it and we have a possible fix that we will run through QA to verify it. You can track the bug here if you wish. CMS-29327

Regarding if changing menus is a breaking change, I dont personally think so. But that can always be debated and all views are valid. We managed to get many addons to work without any change required. This addon should work once we fix the bug. Also the new menu system allows for two levels of menus on the left hand side. Take a look at admin for example. It has two levels. Our reasoning is that an addon that has its own menu can continue to use it. Sure it wont look like the admin menu. But it will work. If a addon wants to use our menu system instead, it can be accomplished with the IMenuProvider and adding subitems to the main item. It will load the second level once the first level is clicked upon. See example below.

image

We know that we have been dropping some large issues lately and we apologize for that. Please know that we take it all very seriously and will continue to try and improve as we move forward.

GeekInTheNorth commented 11 months ago

@DeepRed I had a look at the ticket, I had the same issue with one of my modules in terms of the menu and that does not use an iFrame. The menu change fixed the height of the main div container and I had to apply an overflow to a child container instead.

DeepRed commented 11 months ago

@GeekInTheNorth thanx, good to know. The fix we have looked at is changing the height of the div and changing overflow. We believe it should function for your scenario as well. I will update the description of the bug.

GeekInTheNorth commented 10 months ago

For others that are wanting an interim fix, you can do this yourself. The Admin plugin for the Geta NotFound Handler is a Razor Class Library, which means you can provide an alternate Razor file in your implementation.

So in your web project, create a new razor file with the following path and name: /Areas/GetaNotFoundHandlerAdmin/Pages/Shared/_Layout.cshtml

The following is essentially the same razor file as the one in this package, except with some URL corrections on the menu links to handle the razor file being external to the DLL and the minor UI fix:

@using System.Reflection
@using Microsoft.AspNetCore.Mvc.TagHelpers
@{
    // TODO: Remove this file once the menu fix for Geta Not Found Handler has been released.
    var version = Assembly.GetAssembly(typeof(Geta.NotFoundHandler.Optimizely.ContainerController))?.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion;
    version = version == null ? string.Empty : $"v{version}";
}
<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="utf-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>@ViewData["Title"]</title>

    <link href="https://cdn.jsdelivr.net/npm/bootstrap@5.2.3/dist/css/bootstrap.min.css" rel="stylesheet" integrity="sha384-rbsA2VBKQhggwzxH7pPCaAqO46MgnOM80zW1RWuH61DGLwZJEdK2Kadq2F9CUG65" crossorigin="anonymous">
    <link href="/_content/GetaNotFoundHandlerAdmin/css/dashboard.css" rel="stylesheet">
</head>
<body>
    <header class="navbar navbar-dark sticky-top bg-dark flex-md-nowrap p-0 shadow">
        <a class="navbar-brand col-md-3 col-lg-2 me-0 px-3" href="">NotFound Handler <span class="version">@version</span></a>
        <div class="ml-auto mb-1 px-3">
            <span class="geta-logo-prefix">by</span> @{
                await Html.RenderPartialAsync("Shared/_Logo");
            }
        </div>
        <button class="navbar-toggler position-absolute d-md-none collapsed" type="button" data-bs-toggle="collapse" data-bs-target="#sidebarMenu" aria-controls="sidebarMenu" aria-expanded="false" aria-label="Toggle navigation">
            <span class="navbar-toggler-icon"></span>
        </button>
    </header>
    <div class="container-fluid" style="overflow: auto; height: calc(100vh - 80px);"> // SCROLL FIX IS HERE
        <div class="row">
            <nav id="sidebarMenu" class="col-md-3 col-lg-2 d-md-block bg-light sidebar collapse">
                @{
                    var pageRoute = ViewContext.RouteData.Values["page"].ToString();

                    string Active(string pageName)
                    {
                        if (pageRoute.Contains($"/{pageName}"))
                        {
                            return "active";
                        }
                        return string.Empty;
                    }
                }
                <div class="position-sticky pt-3">
                    <ul class="nav flex-column">
                        <li class="nav-item">
                            <a class="nav-link @Active("Index")" href="/GetaNotFoundHandlerAdmin/">
                                <span data-feather="repeat"></span>
                                Redirects
                            </a>
                        </li>
                        <li class="nav-item">
                            <a class="nav-link @Active("Regex")" href="/GetaNotFoundHandlerAdmin/Regex/">
                                <span data-feather="refresh-cw"></span>
                                Regex Redirects (Beta)
                            </a>
                        </li>
                        <li class="nav-item">
                            <a class="nav-link @Active("Suggestions")" href="/GetaNotFoundHandlerAdmin/Suggestions/">
                                <span data-feather="file-text"></span>
                                Suggestions
                            </a>
                        </li>
                        <li class="nav-item">
                            <a class="nav-link @Active("Ignored")" href="/GetaNotFoundHandlerAdmin/Ignored/">
                                <span data-feather="eye-off"></span>
                                Ignored
                            </a>
                        </li>
                        <li class="nav-item">
                            <a class="nav-link @Active("Deleted")" href="/GetaNotFoundHandlerAdmin/Deleted/">
                                <span data-feather="trash"></span>
                                Deleted
                            </a>
                        </li>
                        <li class="nav-item">
                            <a class="nav-link @Active("Administer")" href="/GetaNotFoundHandlerAdmin/Administer/">
                                <span data-feather="settings"></span>
                                Administer
                            </a>
                        </li>
                    </ul>
                </div>
            </nav>

            <main class="col-md-9 ms-sm-auto col-lg-10 px-md-4 gy-3">
                @RenderBody()
            </main>
        </div>
    </div>

    <script src="https://cdn.jsdelivr.net/npm/bootstrap@5.2.3/dist/js/bootstrap.bundle.min.js" integrity="sha384-kenU1KFdBIe4zVF0s0G1M5b4hcpxyD9F7jL+jjXkk+Q2h455rYXK/7HAuoJl+0I4" crossorigin="anonymous"></script>
    <script src="https://cdn.jsdelivr.net/npm/feather-icons@4.29.0/dist/feather.min.js" integrity="sha256-7kKJWwCLNN8n5rT1MNUpVPkeLxbwe1EZU73jiLdssrI=" crossorigin="anonymous"></script>
    <script src="/_content/GetaNotFoundHandlerAdmin/js/dashboard.js"></script>
    @RenderSection("Scripts", required: false)
</body>
</html>
marisks commented 10 months ago

@GeekInTheNorth Could you please create a PR for this fix?

marisks commented 10 months ago

@DeepRed Changing UI is ok, we expect it, but there is a lack of documentation about how to integrate our UIs in the editorial interface. Also, add-on developers are left with fewer features than it is available for Optimizely (according to Slack discussion). The "icon" menu item and minimal menu are exclusive only to Optimizely. As in your screenshot, double menus take up a lot of space. If an addon requires maximum space for the content, there is no solution now.

There are a few features we need and need them documented:

GeekInTheNorth commented 10 months ago

@marisks It appears we don't need to fix this anymore thanks to Optimizely doing a more global fix to how they handle the menus:

https://world.optimizely.com/documentation/Release-Notes/ReleaseNote/?releaseNoteId=CMS-29327