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

Fixes Placement of dropdown in many scenarios #109

Closed

Conversation

kerwin-wildsea
Copy link

@kerwin-wildsea kerwin-wildsea commented Nov 18, 2024

This PR fixes the dropdown menu misplacement in certain scenarios.


Includes:

  • Using a different render hook which is present on profile pages (including isSimple: true
    • This also fixes a bug when using $switch->outsidePanelRoutes() with $panel->profile(isSimple: false) rendering the language switcher twice as both isVisibleInsidePanels() and isVisibleOutsidePanels return true
  • It changes some styles to fix the 4 (Top|Botom)(Center|Right) outsidePanelPlacement options which all aligned (Top|Bottom)Left.
  • Removal of auth.profile from default outsidePanelRoutes() function as the new render hook is used by both profile styles (simple and non-simple) handling profile as "outside" redundant.


Style changes:

// resources/dist/filament-language-switch.css
-.w-fit{width:-moz-fit-content;width:fit-content}

kerwin-wildsea and others added 7 commits November 18, 2024 11:18
Reason this is problematic, is because profile page is "within panel" when `isSimple: false`. But is "Outside panel" when `isSimple` is set to true (default)
This render hook works and is always considered insidePanel
@kerwin-wildsea
Copy link
Author

I've updated the description after removing most of the "ugly" logic for profile page checking. This seems like a great solution to the problem. If this was already evaluated, please do so again. Thanks 🙂

@kerwin-wildsea kerwin-wildsea changed the base branch from 3.x to main November 26, 2024 09:21
@kerwin-wildsea
Copy link
Author

@bezhanSalleh A friendly reminder for this PR. If there's a reason this hasn't (or cannot) be merged, please let me know 🙂

@bezhanSalleh
Copy link
Owner

I thought i wrote back, my bad! So the reason this PR cannot be merged is, you can achieve the same result without it by just doing the following:

LanguageSwitch::configureUsing(function (LanguageSwitch $switch) {
        $switch
            ...
            ->renderHook('panels::user-menu.before')
            ...;
    });

so i'm gonna close it. I'm currently swampped, though i did figure out how to make it work but that's gonna take awhile. maybe after the holidays.

@kerwin-wildsea
Copy link
Author

I thought i wrote back, my bad!

@bezhanSalleh No worries ❤️

The solution you posted would indeed work. Hopefully it is something you'll eventually are able to get to, but for now it's nice that it can be achieved without hacky-ness!

The other item this PR attempted to fix, was the broken alignment for outsidePanelRoutes. Which is exactly the same as #91.
That can unfortunately not be fixed without using something like vendor patching. If you have some time in the future, maybe that could be merged (#91 is a lot less invasive compared to this PR).

Either way, thanks for the quick response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

outside panel placement always pointing to left side (top, bottom) only
2 participants