-
Notifications
You must be signed in to change notification settings - Fork 105
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
Add subsite change detection #516
Conversation
This looks really useful, thanks! |
0524bc5
to
4261f0d
Compare
@lekoala I've retargetted this at the However, while testing this I found a (probably very common) scenario where this will cause more confusion rather than less. Can you think of an easy way to resolve this in a general way? We could obviously check if someone's in a cms page edit form, but that seems like a narrow scope when this scenario could theoretically happen in other contexts as well. |
@GuySartorelli reloading should load the proper website no? any way, it would be easy i think to pass along the SubsiteID in the url to force loading the last active subsite |
The problem we had in past was that not all model admins or all CMS UI was the same for all subsites, so where should it take you on a reload when you were editing a page type that is not available for the destination subsite? Go to default pages view? |
@michalkleiner fair point. i guess that would lead to a 404 so i'm not even sure how the redirect would work. i'm not saying my approach in 100% bulletproof (it works for me, and that's already something :) ) so maybe that's why it should be opt in? |
I'd hesitate to merge this even as an opt-in feature at the moment, since it doesn't play nicely with editing pages which is arguably the most common action in the CMS. |
I would be ok to have this opt in, if it always took you to just /admin and it clearly said it in the dialog. Or even better with the message customisable, which would require adjustments to source it from config and put it e.g. in a data attribute, but that's all doable. |
@lekoala Are you still interested in having this PR merged in? If the changes Michal suggested in the last comment are made we can accept it. |
still interested but with limited time to spend on it |
I'm going to close this for now since there has been no action for several months - we can reopen it when/if you get time to pick it up again. |
no worries ;) it's just it's working "good enough" for me at the moment so i have limited interest in getting this further |
Closes #515