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

Localize array flip error and title() in models #19

Closed
SomosAMambo opened this issue Mar 24, 2017 · 8 comments
Closed

Localize array flip error and title() in models #19

SomosAMambo opened this issue Mar 24, 2017 · 8 comments
Assignees

Comments

@SomosAMambo
Copy link

SomosAMambo commented Mar 24, 2017

The method localizableLocales() of Localize class is generating an error because Laravels query method "lists" returns a collection, not an array.

Classes\Input\Localize.php, ln 93

Suggested code:

public function localizableLocales() {
	return array_diff_key( // Keep only locales that don't exist in
		Config::get('decoy.site.locales'),
		array_flip($this->other()->lists('locale')->toArray()), // ... the locales of other localizations
			[$this->item->locale => null] // ... and this locale
	); 
}

Also, all translatable models should have a title() method?
In _localize.haml, ln 21 it expects that the model have it:

%a(href=DecoyURL::relative('edit', $model->getKey()))!=$model->title()

And in views\shared\form\display\_locale.php, ln 22:

$label = "<span class='locale-label'>{$label} - Localized as <a href='".DecoyURL::relative('edit', $sibling->getKey())."'>".$sibling->title().'</a></span>';

So we had to do this:

/**
 * Hopefully this is temporary. See: _localize.haml, ln 21
 *
 * @return string
 */
public function title() {
   return $this->title;
}

Isn't preferable that this is an attribute like ->adminTitle (getAdminTitleAttribute() in Base class)?

Suggested code for _localize.haml, ln 21:

%a(href=DecoyURL::relative('edit', $model->getKey()))!=$model->adminTitle

and for views\shared\form\display\_locale.php, ln 22:

$label = "<span class='locale-label'>{$label} - Localized as <a href='".DecoyURL::relative('edit', $sibling->getKey())."'>".$sibling->adminTitle.'</a></span>';
@SomosAMambo SomosAMambo changed the title Localize array flip error Localize array flip error and title() in models Mar 24, 2017
@weotch
Copy link
Member

weotch commented Mar 24, 2017

@brokenhd Lets add some tests related to localization features. What @SomosAMambo has discovered (thank you @SomosAMambo ) is some issues related to changes between Laravel 4 and 5 that aren't being caught in the current tests.

@weotch weotch self-assigned this Mar 24, 2017
weotch added a commit that referenced this issue Mar 27, 2017
@weotch weotch closed this as completed in f5de62b Mar 27, 2017
@weotch
Copy link
Member

weotch commented Mar 27, 2017

Thanks again, your diagnosis was spot on

@weotch
Copy link
Member

weotch commented Mar 27, 2017

These (and are other issues) are legacies of us not doing a localized project since we released 5.0 (and supported Laravel 5). The tests we're adding now will prevent future breaks.

@SomosAMambo
Copy link
Author

Ok! Let us help!

@weotch
Copy link
Member

weotch commented Mar 27, 2017

I think the best way you can help, besides filling the issues you have been, is if you discover anything not documented or you can think better ways to explain how to use Decoy, submit PRs to the docs: https://github.com/BKWLD/decoy/tree/master/docs. Thanks for offering!

@SomosAMambo
Copy link
Author

There are some undocumented features I would like to help, but I'm digesting all the code first. E.g. The wysiwyg field; apparently there is a way to upload images directly from it, but I was not able to find the documentation for it.

@weotch
Copy link
Member

weotch commented Mar 28, 2017

Yeah, that could use a better implementation. Before we open sourced the repo, our projects would build the assets (js/css) with the project's webpack/grunt. So, the way this is done is a legacy of that and is awkward as a result.

See http://docs.decoy.bukwild.com/customize. You'll need to build your own JS file that consumes Decoy's JS and then call wysiwyg.config.allowUploads() on it. You can see in the examples, this is how you can customize Redactor.

Instead of this, I think Decoy should expose an api on the window so you could just do window.decoy.config.allowUploads() ... probably a pretty trivial change honestly.

@SomosAMambo
Copy link
Author

I'll give it a try! Thanks!

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

No branches or pull requests

2 participants