-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Link to parent menu item #8225
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
base: master
Are you sure you want to change the base?
Link to parent menu item #8225
Conversation
4775e2f to
358317b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8225 +/- ##
=======================================
Coverage 99.08% 99.08%
=======================================
Files 139 139
Lines 4046 4046
=======================================
Hits 4009 4009
Misses 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8349e5e to
b36fd5a
Compare
def150a to
48f2861
Compare
|
This is made to sound like it's considered a bug but from what I know this is intentional. It wasn't supported previously. Also, I don't think it's a good default for the parent menu item to have a link and a separate toggle. I will not accept this change, sorry. Although, you should be able to make this change for your own implementation, that's why we have the navigation in a partial so you can customize it. For that, it seems you would need a Ruby method update to support that case which is something I would accept. You're welcome to contribute just that change here. The JS is only meant as a default, out of the box. You can supply your own JS for custom menu behavior by just changing the element id and working off that. Perhaps, in the future we can expand on the JS so you don't have to import/load what you end up replacing or don't use. I would appreciate any help or suggestions around that for a v4 release. |
|
@javierjulio Actually, this was supported in version 3. Take a look at the following snippet in the Without the change in this pull request, there is no way for you to navigate to the |
|
@hasghari sorry, I don't use that feature. I'm not familiar with it at all. Nor do we seem to have it in the generated app for development or test. Often times these features are latter discovered unless built in to the development app. It's not possible for us to know all of them. Can you please modify the dev app that is generated locally to use that feature and share what changes you had to do to get that? I'm not able to replicate this. I don't have any idea what it would even render. Thank you. |
93cf695 to
5b59de1
Compare
|
@javierjulio I added a new nested resource in the generated app called |
The PR LTGM, I use this feature so thanks for fixing it. |
|
@javierjulio Is it possible to get a fix for this in, hopefully we have changed your mind on this feature :) |
5b59de1 to
6ed5a79
Compare
6ed5a79 to
2eb84b4
Compare
|
@javierjulio Sorry to ping again, but this is a really wanted feature and honestly a blocker for upgrading |
2eb84b4 to
f112e59
Compare
1786e40 to
c1ed616
Compare
|
Hi guys! I bumped into the same issue #8245 Do u need any help with the PR? I'd be really happy to get this thing merged |
c1ed616 to
db18096
Compare
db18096 to
1653971
Compare
|
Cross posting a workaround #8245 (comment) |
1653971 to
94ba87c
Compare
94ba87c to
a2220e2
Compare
a2220e2 to
f8a750a
Compare
f8a750a to
8037685
Compare
8037685 to
7ef5ea8
Compare
7ef5ea8 to
663c4cd
Compare
663c4cd to
8389a4a
Compare
8389a4a to
e6ea1c0
Compare
e6ea1c0 to
d4936af
Compare
d4936af to
648194a
Compare
|
@hasghari we will merge #8709 first to bump Tailwind from v3 to v4 then we'll come back to this. Nothing to do for now until #8709 is merged. I'm not sure if the major version affects your changes, it may not, but feel free to test yourself if you like. Once #8709 is merged, I'll test this and suggest any changes. Both updates are planned to go in the next beta release. Thank you for your patience. |
648194a to
74ba0f9
Compare
Currently when rendering a nested menu, if the parent menu item links to a url, the link is not rendered for the parent item and there is no way to navigate to it.
74ba0f9 to
480f03e
Compare
Currently when rendering a nested menu, if the parent menu item links to a url, the link is not rendered for the parent item and there is no way to navigate to it.
Here is the nested menu appearance for light mode and dark mode
I'm not a designer by any means so I'm open to changing the UI.