The admin UI needs to be overhauled now that we've got a new perspective in skins (and we've gotten rid of .info and skinsets).
Once that's done we'll have to write code for it.

CommentFileSizeAuthor
#28 patch_commit_38312265aa01.patch57.27 KBmoonray
#9 Screen shot 2010-12-08 at 15.47.45 .png169.74 KBmoonray
#6 skinr-skins-page-2.jpg78.83 KBnomonstersinme
#3 skinr-skins-page.jpg84.43 KBnomonstersinme

Comments

moonray’s picture

moonray commented

Some initial comments by Jacine from another thread:

Table on admin/appearance/skinr/skins

This table is way too verbose, and there are too many theme functions used to generate it. I realize we'll have to figure out how to better display this information, but in the meantime, the following tweaks should be made:

  1. Run "theme hooks" through theme_item_list(). Give it #theme => item_list__skinr.
  2. Run "operations" though theme_links(). Give it a #theme => links__skinr and give it an .links & .inline class. Also, make sure you check if $skin_info['links'][$key] is set before doing anything with it.
  3. Nix the <h2> for the skin name.
  4. Remove the description. It's totally wrong in this context.
  5. Change "source name" to Source and just leave the module or theme name there. A module and theme cannot be named the same so this should be enough.
  6. Obviously, remove the "source type" column, per 5.
  7. Get rid of the "source" prefix for version.
  8. Also, if you are going to add classes to the table cells, make sure to do it for all of them so it's consistent.

One last thing regarding that page... The default fieldset has no legend. It should be labeled "General."

moonray’s picture

moonray commented

Here's another issue that is related: #954710: Show a more descriptive title for each setting on admin/appearance/skinr

nomonstersinme’s picture

nomonstersinme commented
StatusFileSize
new skinr-skins-page.jpg84.43 KB

Attached is my proposed UI screen for the skins page.. this is based on the admin theme i created so obviously the styling isn't whats important here.

jacine’s picture

jacine
she/her
Location New York
commented
Status: Active » Needs work

This is almost exactly what I was thinking... :)

3 things:

  1. The checkbox should be in a column called status, and the redundant column should be removed.
  2. The plugin name needs to be listed in the table too.
  3. We need to display empty groups with a message that nothing is in them, so people can drag skins to them.
jacine’s picture

jacine
she/her
Location New York
commented

Oh, and the description needs to go. The context is wrong here.

nomonstersinme’s picture

nomonstersinme commented
StatusFileSize
new skinr-skins-page-2.jpg78.83 KB

Ok i removed the description, changed the title of the checkbox row to status and removed the other column and added the plugin name.

ChrisBryant’s picture

ChrisBryant commented

Looks great to me, thanks!

jacine’s picture

jacine
she/her
Location New York
commented

Ok, cool, looks good.

Thinking more about the status, we should probably use "Enabled" to be consistent with the way the menu menu does this for menu items. I also wonder whether or not it would be better just to name "Theme/Module" source, and in the cells have "Orange theme" or "Whatever module". Plugin name, can also just be plugin.

This is good though, as far as mockups are concerned. I'll work on coding this Friday.

moonray’s picture

moonray commented
StatusFileSize
new Screen shot 2010-12-08 at 15.47.45 .png169.74 KB

A few things:

  1. Status can't be represented as a single checkbox... there needs to be individual status for each enabled theme (so skin A can be enabled for theme 1 but not theme 2).
  2. Do we want to add anything to indicate whether it is a theme or a module?
  3. Do we want to add version info (from the module or theme)?
  4. Do we want to add which theme hooks the skin can apply to? (see attached screenshot for current implementation)
jacine’s picture

jacine
she/her
Location New York
commented

Status can't be represented as a single checkbox... there needs to be individual status for each enabled theme (so skin A can be enabled for theme 1 but not theme 2).

I don't see why we have to expose this here. Enabling a skin can mean that it's enabled for all enabled themes, and we don't have to totally complicate things and confuse people.

Do we want to add anything to indicate whether it is a theme or a module?

I mentioned this above. I think "Source" and "Orange theme" will suffice.

Do we want to add version info (from the module or theme)?

I don't see any reason to do this. We should keep this simple.

Do we want to add which theme hooks the skin can apply to? (see attached screenshot for current implementation)

We never did this before, and I don't think we should do it in this table. What I mentioned to Amanda, is that a "details" link or something that shows a preview of the rendered skin widget and full information about the skin would be preferable.

jacine’s picture

jacine
she/her
Location New York
commented

If we do need to separate this by theme, I still think the table shouldn't change.

Maybe this belongs in the configuration section where we can make proper use of primary and secondary tasks. This should follow the lead of the blocks UI, which means that any separate theme operations should happen on a different tab, but the page and table contents itself should look the same.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Active

@moonray: Could you post a patch of what you have?

jacine’s picture

jacine
she/her
Location New York
commented

I was supposed to try create this patch. I haven't gotten there yet.

moonray’s picture

moonray commented

In order to have a separate tab per theme we'd have to have the following path structure:

admin/skinr

admin/skinr/skins = admin/skinr/skins/list
admin/skinr/skins/list
admin/skinr/skins/edit
admin/skinr/skins/delete

admin/skinr/skin-info = admin/skinr/skin-info/{current theme}
admin/skinr/skin-info/bartik
admin/skinr/skin-info/seven

admin/skinr/rules

admin/skinr/tools
admin/skinr/tools/import
admin/skinr/tools/export

However, that would mean Skinr would be at the top level in admin (alongside content, structure, appearance, etc.) I'm not sure that's the best way to do it.
I would like some additional proposals.

EDIT: This is in response to #11

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Active » Postponed

It doesn't make sense to work on this right now. We need to (dirty/quick) fix the actual problems at hand to allow people to at least use Skinr (e.g., #977118: Skins in disabled basetheme cannot be enabled for basetheme (to appear in subtheme)). And after or in parallel to that, we need to rewrite the entire configuration and loading of skins (#1029058: [META ISSUE] Change skin configuration).

Only after those issues, we'll actually have a clue about what remains for the administrative UI, and how it should ideally look like.

jacine’s picture

jacine
she/her
Location New York
commented

Why not?

We really, really need to get a move on here and get to a point where we can have a usable release. This one issue at a time thing is just not working. I can't go along with it anymore unless we have a real roadmap in place. I knew we wouldn't be ready for Drupal 7, but the pace here is dismal and we will never ever get this done unless we change that.

Rewriting the entire configuration of skins could take ages. It's not fair to everyone that relies on this module. It doesn't have to be perfect for a working alpha version. We don't have the time or resources for perfection.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Isn't that exactly what I stated...?

We need to (dirty/quick) fix the actual problems at hand to allow people to at least use Skinr

Users are currently blocked, because they cannot use Skinr. So it would make sense to fix the broken parts in order to create a very early and first alpha for D7, so people who really need it can actually use it. Of course, drafting a new admin UI could happen in parallel, but it doesn't really add value to the aforementioned, current task at hand. At least, that's my perception and recommendation. You naturally have the mighty powers to overrule me. ;)

jacine’s picture

jacine
she/her
Location New York
commented

But this is a broken part like the rest of the UI that used to work, and that users need to operate Skinr properly. Without the admin UI in a working state, users cannot manage skins without coding. There also used to be a page with a list of applied settings and links to edit them, but that is also not working since we switched skins over the PHP. That stuff needs to be fixed, just as much as the code needs a rewrite. In this case that requires some changes.

I'm not saying that the other issues shouldn't be fixed as well. What I'm saying is that there is no harm in doing more than one of these tasks at the same time, and that if we want to get to the point where we are ready for a release multi-tasking is going to have to happen.

The biggest problem here is that no one knows what the next tasks should be at any given time, so when time is available to work on this project nothing happens. That's just sad and totally frustrating. This issue is one of the few things, IMO that we can work on. Are there other things that can be done right now? Probably, but I honestly have no idea what they are (and neither does moonray), so that's why I jump to this issue - To do something.

I have no idea what is required to get to a release here, and I desperately want to get there, but I can't work like this. There needs to be a roadmap of what needs to be done and we need to figure out how to split up the work and get there. If not, we are going to continue going nowhere fast.

rickmanelius’s picture

rickmanelius commented

One of the hardest things I'm facing with the skinr module is the lexicon. In the pictures linked to above, what is the 'plugin' mean? Is that related to a skinset? A replacement name for that?

jacine’s picture

jacine
she/her
Location New York
commented

@rickman The documentation is still in progress (and we could use help finishing up), but a lot of what you are probably looking for is already here: http://skinr.org/documentation. In short, Skinr plug-ins are what skinsets used to be. To make any skin in Drupal 7, you need to implement a skin plugin. There's also an overview of the entire process on the home page.

rickmanelius’s picture

rickmanelius commented

@jacine.
Thanks for the clarification. I actually like 'skinsets' better than plugins personally as it seems more descriptive of what is contains... but I'll learn to love plugins!

As for the skinr.org site. I didn't know that existed! Perhaps putting a link on the module front page would help direct more people over there as I wasn't even able to find it via googling.

BTW. No worries on the documentation. I know how hard it is to keep up when so much time is needed to code!

moonray’s picture

moonray commented
Status: Postponed » Active

Now that most of our major back-end stuff is in, we need to start fixing the admin UI.

nomonstersinme’s picture

nomonstersinme commented

responding to #14:

i second this idea and menu structure... skinr doesn't seem to fit within any of the admin top level items so why not break it out into its own thing.

I can't think of any other proposals for menu structure but the rules page should show more information on the rule like on the skinr list page, imo..

stephthegeek’s picture

stephthegeek commented

I think Skinr fits naturally under Appearance, but per #14, it can't go there if we want different configuration per theme?

I know creating your own top level items is frowned upon but I guess we don't have much choice?

nomonstersinme’s picture

nomonstersinme commented

I disagree.. skinr has always felt a bit awkward under appearance. no other modules that i've come across use appearance for the administration links.. even modules that alter the appearance of content like display suite. If we put skinr in structure we can make use of the primary and secondary tasks and show plugin info by theme. i think this would be a good solution.

Also I think if we're calling skins plugin's now that should be consistent in the admin, so as to not confuse people further.

admin/structure/skinr = admin/structure/skinr/skins = admin/structure/skinr/skins/list  <--right?? or what else would show here..
admin/structure/skinr/skins/list
admin/structure/skinr/skins/edit
admin/structure/skinr/skins/delete

admin/structure/skinr/plugins = admin/structure/skinr/plugins/{current theme}
admin/structure/skinr/plugins/bartik
admin/structure/skinr/plugins/seven

admin/structure/skinr/rules

admin/structure/skinr/tools
admin/structure/skinr/tools/import
admin/structure/skinr/tools/export
moonray’s picture

moonray commented

@nomonstersinme: I mostly agree with you. A few changes, though...

admin/structure/skinr  <-- list details of the subsections for skinr, like admin/structure does.

admin/structure/skinr/skins/import <-- import and export should be with the configured skins
admin/structure/skinr/skins/export

In addition, I'm not sure I like using plugins as the path for where we enable/disable skins (not applied skin configuration). #1051560: Generate new skin plugins will allow you to edit plugins, which will confuse these two areas.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

What you want is:

admin/structure/skinr
admin/structure/skinr/list == admin/structure/skinr
admin/structure/skinr/add
admin/structure/skinr/skin/%/edit
admin/structure/skinr/skin/%/delete
admin/structure/skinr/skin/%/export
admin/structure/skinr/import

admin/structure/skinr/plugins
admin/structure/skinr/plugins/bartik
admin/structure/skinr/plugins/seven
admin/structure/skinr/plugins/convert

admin/structure/skinr/rules

However,

- "plugins" indeed feels a bit poorly named. "available" or similar might be better.
- not sure about the future of "rules"

moonray’s picture

moonray commented
StatusFileSize
new patch_commit_38312265aa01.patch57.27 KB

Committed the following patch.

It changes most of the paths as per #27, and updates the admin/structure/skinr/plugins section (which is now admin/structure/skinr/library).
Tests have been updated accordingly as well.

If you have any better suggestions for path filenames, please pipe up.

I'm leaving this issue open since there's quite a lot more we can do with the admin section. But this, the most critical, at least has been dealt with.

ajross’s picture

ajross commented

I think it would be helpful to update the documentation at skinr.org. Under Step 4 of the slideshow (Enable your plugin) it says to go to admin/appearance/skinr/skins to enable your skin. I expect people using the latest 7.x version will be confused when there is nothing related to skinr under appearance, and may not immediately think to check under admin/structure.

moonray’s picture

moonray commented

Skinr.org has been updated with the new paths.

moonray’s picture

moonray commented

Proposed changes to admin/structure/skinr/library (summary from above) that still haven't been implemented.

  1. Remove description. The context is wrong.
  2. Add a "details" link or something that shows a preview of the rendered skin widget and full information about the skin. Move the theme hooks info into there.

What else needs tweaking, still in the admin section?

moonray’s picture

moonray commented

Took care of 1. Remove description. The context is wrong..

2. Add a "details" link or something that shows a preview of the rendered skin widget and full information about the skin. Move the theme hooks info into there.

This will need some discussion, examples, requirements.

moonray’s picture

moonray commented
Title: Re-design Skinr admin UI » Add "details" link that shows preview of rendered skin widget and additional info
Issue summary: View changes

Updated the task description.

moonray’s picture

moonray commented
Priority: Major » Normal
Status: Active » Postponed (maintainer needs more info)

  • moonray committed acd505b on 8.x-2.x 
    Issue #984402 by moonray: Updated Skinr admin UI.
  • moonray committed 95743f5 on 8.x-2.x
    Issue #984402 by moonray: Re-design Skinr admin UI.
astonvictor’s picture

astonvictor commented
Status: Postponed (maintainer needs more info) » Closed (outdated)

D7 reached its EOL back in January 2025, and there is no active release for D7 for this module anymore.
Development or support is not planned for D7. All D7-related issues are marked as outdated in a bunch.

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.