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

groups: group specific css tags (Trac #5029) #5029

Closed
elgg-gitbot opened this issue Feb 16, 2013 · 7 comments
Closed

groups: group specific css tags (Trac #5029) #5029

elgg-gitbot opened this issue Feb 16, 2013 · 7 comments

Comments

@elgg-gitbot
Copy link

Original ticket http://trac.elgg.org/ticket/5029 on 43098376-08-24 by trac user tunist, assigned to unknown.

Elgg version: 1.8

the group_custom_layout plugin (http://community.elgg.org/plugins/609957/1.3.5/group-custom-layout-the-latest-version-is-updated-for-elgg-176) was quite popular in elgg 1.7 series.
i am regularly asked to update it to 1.8.

i realise that the better solution is to adapt the core groups plugin to support custom, unique css tags - per page item and per group; this would allow themes to include stylisation that is unique for each group on the site - or at least for specific groups that the site admins wish to theme uniquely.
each styled html object on a group page could receive an additional tag to identify it as belonging to a unique group.

e.g. the page body div:

would become:

which would allow subelements to be called/themes.

ideally all elements on group pages would be tagged in this way.

@elgg-gitbot
Copy link
Author

cash wrote on 43098813-01-23

Option 1 would be putting a css class like "groups-" in the main profile view (https://github.com/Elgg/Elgg/blob/master/mod/groups/views/default/groups/profile/summary.php)

Option 2 would be passing a class into elgg_view_layout() from the group page handler.

Option 3 would be always adding a class to the layout/page for the page owner.

@propertunist
Copy link

i am not familiar enough with the code to know which is the best solution.. just know that the class needs to include a unique id for each group.. so.. .elgg-groups-mygroup1 etc.

@jdalsem
Copy link
Member

jdalsem commented Oct 20, 2014

i do not believe it to be best practice to add guid to elements to style them differently. If you need custom style based on group type or guid (or some other differentiation), you could easily override the groups/profile/layout to add the wrapper class.

@jdalsem jdalsem closed this as completed Oct 20, 2014
@propertunist
Copy link

why not add that wrapper to core?

@jdalsem
Copy link
Member

jdalsem commented Oct 21, 2014

because there is no general logic to add this class... maybe you want it per guid, but someone else wants it based on the first letter of the group name and someone else wants it based on a certain group profile file or even member count...

@propertunist
Copy link

since no version of this feature currently exists, there is nothing lost by adding it in. if someone else decides that they want to alter the structure of the css class name then they can do by over-riding as you already pointed to. they are not likely to do that if the wrapper doesn't exist at all, since they may never even think of the idea as something they may want to do.

@ewinslow
Copy link
Contributor

This is a niche feature request. I'm guessing 99% of Elgg sites will not need this. On top of that it's possible to do via a more general API (I.e. via plugin).

The thing that we lose by adding features like this is flexibility. We have to maintain and support them for a long time. We have to consider BC. Therefore it's better if we're just brutal about which features make the cut and not add any in unless they're valuable to a large percentage.

We weren't always so harsh about what features got added, but I'd argue we're suffering for it now.

I'm sorry if this makes life a little more difficult for you.

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

No branches or pull requests

4 participants