-
Notifications
You must be signed in to change notification settings - Fork 116
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 options for tabs within the more actions dropdown #323
Comments
For reference we worked on a similar component which never quite made it in, before the MoreActions React component was available in core: symbiote/silverstripe-gridfieldextensions#225 |
Think about generating a cache on dev/build / ?flush (implement |
@clarkepaul How important is this feature, compared to spending this time on other features, or having a slower CMS experience with a "simplistic" implementation? How likely do you think it is that authors navigate from the block overview into a specific tab via the "meatballs" menu, vs. just clicking "edit block" to get to the detail view, and then selecting the tab there? Has this shortcut been something out of user testing? Update: Also, we'll be looking at caching all form fields for all record types in the CMS at some point, to speed up the overall authoring experience. This would also include tabs, and can be used for the "meatballs menu" as designed here. This would make any short-term solutions obsolete. Could we wait until we're in a position to do this properly? |
Hey @chillu. The designs currently do not indicate it is possible to get to the GridFieldDetailForm if the block is in-line editable. The only options are the tabs in this meatballs menu, the tabs rendered as "normal" - inside the in-line form, or as you say, providing the ability to access the edit (GridFieldDetailForm) screen and using the tabs that way. |
I’m assuming that most content blocks will have a subset of form fields being inline editable (e.g. just title and content), and then all fields in the detail edit form? And those inline editable fields might contain fields across tabs (depending on priority), but there’s no tabs directly in the inline editing. We have to create a solution which works for If we're loading the full Apart from that, having a view mode toggle (tabs) hidden away in a "more options" dialog seems like a weird UX choice. After selecting a different view mode (different tab), you have no way of knowing you're in this mode without opening the menu/dialog again. And if we introduce a separate "current tab" viewer, it begs the question why we're not just using a GridField detail form here. Keep in mind that the eventual goal is for this detail edit form to load faster (caching form fields), so the mode switch won't be as painful for authors. /cc @clarkepaul |
@chillu the current route being taken is for content blocks to be inline editable or not. Being built with an opt-out approach, any block that is set to be inline editable would by default display everything from the Let's pause on this issue to make sure the performance impact is released and clarify the UX desire behind it. |
Lots going on in this conversation so I'll try to recap some of the reasoning.
I think the other tabs within the dropdown will be used far less than editing "content". Therefor easy access to the content tab was far more important.
Nope, the in-block editing view represents all the fields in the "Content" tab as being editable. Blocks naturally grow to the height of the fields inside. We are condensing groups of fields into single click areas to reduce consumed height (like a link which has: link text, link, title, new tab...). I'm trying to keep this as basic as possible (all fields treated the same). Tabs will transfer to blocks with the same structure (tab for tab), and nested tabs will continue to look like tabs with the in-block edit view. I can't recall the exact details of the teams discussion on performance but pretty sure we had some options (hopefully captured somewhere), for example only loading the fields within a block when its expanded could be expected. |
Just to make it clear with a bullet pointed list of pertinent details:
Three options (in order of design preference I guess): We can't do the most preferable option without some sort of cache for the output of I'm still keen to do option A - I think the design benefit outweighs the technical debt, which we can minimise by containing it (And this is already estimated for it - only a 3). |
Sorry, what is the |
@NightJar I updated my comment to be more specific. The I'm not sure what you're referring to with difference from |
Yeah thanks for clarifying. In which case putting the tabs into the |
Alright, thanks for taking the time to explain this - an important nuance that I didn't pick up on when reviewing the designs. But my fault for not being more involved in the discussion which lead to this point. Essentially, we're forming developer habits here, one way or another. I'd rather push them towards not having tabs on inline editable tabs than creating workarounds for them. But devs sometimes don't control the presence of tabs (e.g. auto-added "link tracking" tabs), so this seems like a good compromise. Would
That would be my preference, since we don't need to come up with a short-term solution for getCMSFields caching. How hard is that? Even with the full "form field client-side cache" in place, that would only happen on demand - the first time the detail edit form (or inline edit form) is requested for this particular model. Which might be when you expand the block, not when you display its summary (TBC). |
I see the "(TBC)" but I'll address some of your statements already:
TL;DR: Yes.
This will be when you expand the block. In this case - we still want the short-term solution for caching tabs because we won't have any source of data until the block is expanded (but the tabs should appear in the "More Actions" menu from the get-go). Updating tab options dynamically when the form schema is loaded is something we should definitely do. |
So either you have to pre-load all cms fields for all blocks (before the first one could be expanded), which would be slow. Even if done async in the background, that's a lot of CPU cycles on the server. Or deal with a "more options" menu that doesn't have the tabs until you first expand each type - at which point you can implement client-side caching. My point is that I don't want to create a system which does the pre-loading without considering the bigger picture (GraphQL-based form schemas which are hopefully cached through Apollo). And if we wait until the first expand action, we might as well dynamically append those, and deal with caching properly later across all form fields incl. tabs. |
If we decide to not show tabs until the block is expanded, that would be amazing. If @clarkepaul is on board we don't really have a problem. I agree that it's loads of CPU cycles on the server - hence the intention was to have a long-term server-side cache (that is cleared on flush). I agree that creating a system in elemental just for this is not ideal but does have some advantages over Apollo caching (as I see it):
Basically it's a minimum effort required to solve the problem with the fastest (perfomance-wise) solution that doesn't introduce hoards of technical debt. I'm keen to produce some code as a POC. I've self-assigned this but I won't actually do anything till we've resolved these discussions. |
Are you suggesting that we have only actions in the dropdown when the block is collapsed, but if the block is expanded it has both the tab links and the block actions? Its a bit hard for me to know the impact of the intended UI. If you find that development is blowing out because of showing the tabs in the dropdown menu, then we could hide the [...] icon until the block is expanded—as long as its the intention we continue to enhance the experience at a later date. I think easy access to links is going to reduce clicking, especially when it comes to saving/publishing blocks. |
FYI @chillu - Here's the server side caching I implemented in the PR: https://github.com/dnadesign/silverstripe-elemental/blob/f1162762c6ef30a61b550fc823f3100e9b1a5b51/src/Services/ElementTabProvider.php |
MVP implemented in #373 I think this is good enough to close this ticket. The expensive operations are done on flush and cached, so there's negligible impact to the CMS user. @ScopeyNZ has also done a nice job of hiding away that part of the logic in an API marked as internal in case we come up with a better way to do it in future. |
The designs indicate that the tab options that usually appear in a
GridFieldDetailForm
for blocks will need to be available in a dropdown on blocks. These need to be available per block without considerable performance degradation.To handle this, I propose that we check tabs per block type (extension of
BaseElement
) and cache the result. There's is a possibility that the data saved on a block may affect the tabs that are shown, but when a block is expanded we can update the tab options dynamically (logged at #324).A/Cs:
PR: #373
The text was updated successfully, but these errors were encountered: