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.

Sign up for GitHub

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

Jump to bottom

Some bugs and flaws: renaming, deleting and translation #2

Open
yomli opened this issue Jul 16, 2015 · 0 comments
Open

Some bugs and flaws: renaming, deleting and translation #2

yomli opened this issue Jul 16, 2015 · 0 comments

Comments

@yomli
Copy link

yomli commented Jul 16, 2015

Hi,
First of all, thank you for the work you've done. It's a great plugin. While using it as a base for my own backends, I noticed some bugs/flaws :

Renaming assets

Renaming a picture change its file by its thumbnail. It was funny to see. In do_rename() :

// Comment that and replace it
// $pathAbsoluteThumbNew = str_replace($filename, 'thumbs/' . $filename, $pathAbsoluteNew);
$pathAbsoluteThumbNew = str_replace($filename, 'thumbs/' . $filenameNew, $pathAbsoluteOld);

Deleting

I noticed some flaws with deleting markdown files or assets. E.g. for assets, Pico Admin deletes the file, but not the thumbnail (and doesn't edit the meta.php file). And markdown files aren't deleted at all. So I suggest a modification of do_delete() :

if( !$isDirectory ) {
    // Not mandatory since we will sort the files after
    // if( strstr($file_url, '.') === false ) {
            // We check if the file is a markdown file
        $fileMarkdown = $file . CONTENT_EXT;
        if( file_exists(CONTENT_DIR . $pathFile . $fileMarkdown) ) {
                // Delete markdown file
            die( unlink(CONTENT_DIR . $pathFile . $fileMarkdown) );
        //}
        } else {
                // Delete asset file
            if( is_file($file_url)) {
                unlink($file_url);
                    // Update meta.php
                $path     = str_replace($file, '', $file_url);
                if( file_exists($path . 'meta.php') ) {
                    $meta = array();
                    include($path . 'meta.php');

                    if( array_key_exists($file, $meta)) {
                        $attributes = $meta[$filenameOld];
                        unset($meta[$file]);
                        file_put_contents($path . 'meta.php', '<?php $meta = ' . var_export($meta, true) . ';');
                    }
                }
                    // Delete thumb file
                if ( file_exists($path . '/thumbs' . $file) ) {
                    unlink($path . '/thumbs' . $file);
                }
            }
        }
} else {

Translation

Errors

Errors are not translated, nor displayed. I edited several functions to return errors with the ajax response so I can display them with alert() (like you've done with do_new()). For the translation part, I wrote a simple function :

/*
 * Translate messages of this file as well.
 */
private function lang($message = 'global') {
    return Pico_Admin_Helper_Strings::translate('trans.' . $message);
}

I just call it with $this->lang('error.move.failed') and all the error messages are in the translation files. Pretty simple.

lang="en"

In the translation file, add :

$translations = array(
    'global'                    => 'en',

So we can edit the templates to use it :

<html lang="trans.global">

This way, the spell checker of the browser can work with the good language, which is great for the editor content.

pico_admin.js

Renaming assets

Why do you let the user change the extension? This just causes an unnecessary error. My suggested rename function for assets :

rename: function(pathFileFull, isDirectory) {
    var filename = pathFileFull.substr(pathFileFull.lastIndexOf("/") + 1);

    // We trim the extension. PHP won't let it pass anyway
    var ext = "." + filename.split(".").pop();
    filename = filename.replace(/\.[^/.]+$/, "");

    var nameNew = prompt( (isDirectory ? PicoAdmin.labels.promptRenameDirectory : PicoAdmin.labels.promptRenameFile), filename);
    nameNew += ext;

    if( nameNew ) {
        $.post('admin/rename', { file: pathFileFull, renameTo: nameNew }, function(data) {
            // Reload assets manager
            PicoAdmin.AssetsManager.init( PicoAdmin.AssetsManager.fileUrl );
        });
    }
},

I saw others issues, but I was refactoring PicoAdmin for my own use, so I didn't write all of them down. Environments used: local and distant LAMP (Gandi Simple Hosting for the distant, Bitnami for the local).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@yomli