Add the Type Tray module #77
Conversation
We have been successfully using Type Tray in all of our projects.
| type_tray: | ||
| type_category: _none | ||
| type_thumbnail: '' | ||
| type_icon: /sites/default/files/type-tray-icons/icon-blog-post.svg |
There was a problem hiding this comment.
🤔 My only concern here is that, if and when we add the ability to install the site in a directory other than sites/default, this will break.
So we either need test coverage that the icons actually appear, or we need to find a way to make this a little more dynamic. A custom config action, maybe? IMHO, the correct resolution here is for Type Tray to support more dynamism in this setting, if there's not already an issue for that. (If we were able to just use the public:// stream wrapper, this problem would go away. Maybe we already can do that...?)
That said, I'm okay deferring this to a follow-up PR. (Can you open an issue for it here?)
|
Oh, one other request, @e0ipso -- in another PR, can you opt the Basic block type into this as well? It will necessitate creating a new |
|
I was initially on the fence about this, but I tried it out and it is an improvement. I wasn't sure about needing an icon for each content type, but I see there is a default icon that is perfectly fine. I would also just say the basic page icon proposed here doesn't really make sense to me, but it's not super important. I also think the styling could use some love but that's what's great about Starshot, it should drive improvement in contrib as well :) |
I agree on this. But that's very easily fixed later. I think my only remaining concern here is the possibility of this breaking if the site directory is not |
|
|
|
I think it can be a follow-up as long as we have an issue open -- ideally in Type Tray's issue queue, with a follow-up issue here to use the fix when there's a patch. @e0ipso, if you open those things, we can merge this. |
|
Opened an issue for this, with a patch (no tests yet): https://www.drupal.org/project/type_tray/issues/3447694 Can we apply that patch here, and switch to using stream wrappers? |
|
Sorry for taking this long to reply. I will be looking into this today. |
|
@phenaproxima @pameeela this was fixed. Let me know if there is anything else. |
|
Thanks @e0ipso et. al., this looks great. |
|
I'm wondering about adding this now in light of a few other ongoing discussions about how to vet modules. This one is (pretty much) purely a visual change, not really functional. It does offer categorisation to content types, but that is unlikely to be useful for the Starshot audience I think, because it would only be needed if there are a lot of content types. I still really like the module but now I'm second guessing the addition! |
|
It's true that Type Tray's changes are visual, but IMHO they are visual in a way that increases Drupal's user-friendliness and usability, particularly by supporting type-specific icons. That's why I added it. I think it's completely fine and in line with Starshot's mission to add "cosmetic" modules if they enhance usability. That's why I'm on the fence about Gin Login, which absolutely is a visual upgrade, but not necessarily a usability upgrade. |
|
Which modules to add, and which not to add ... it's a fine balance. There are nice-to-have modules, and then there are modules which add a lot of value. Every "nice" module included risk adding technical debt, and -- by definition -- will increase the number of modules, at the risk of ballooning into a sea of modules, if not kept in check. Keeping the number of modules at a reasonable level is good practice, in my opinion. |
We have been successfully using Type Tray in all of our projects.
The reasoning behind using it is documented here.