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

Prettify model preview page #781

Merged
merged 1 commit into from
Sep 28, 2017
Merged

Prettify model preview page #781

merged 1 commit into from
Sep 28, 2017

Conversation

AbbyJanke
Copy link
Contributor

As discussed in PR #750 simplified the preview page. Automatically pulls all columns from the database using setFromDB() removes any previously set columns within setup() function.

Also added small fix for setFromDB() to solve DBAL to fix enum issue. Code from issue #236.

Added small fix for DBAL to fix enum issue. run setFromDB and remove previously set columns.
Copy link
Contributor

@lloy0076 lloy0076 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces after ifs!

// cycle through old columns for removal
foreach($this->crud->columns as $old_id => $column) {
// replace any relationship columns
if(array_key_exists('model', $column)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space after if!!!

Why exists vs isset?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! Thanks.

// get the info for that entry
$this->data['entry'] = $this->crud->getEntry($id);
$this->data['crud'] = $this->crud;
$this->data['title'] = trans('backpack::crud.preview').' '.$this->crud->entity_name;

// remove preview button from stack:line
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? I forget why...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you're already on the "preview" page :-)

@@ -3,7 +3,7 @@
@section('content-header')
<section class="content-header">
<h1>
{{ trans('backpack::crud.preview') }} <span>{{ $crud->entity_name }}</span>
{{ trans('backpack::crud.preview') }} <span class="text-lowercase">{{ $crud->entity_name }}</span>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have text-lowercase in that span? German, for one, has rules about capitalising nouns for example?

@@ -15,19 +15,50 @@

@section('content')
@if ($crud->hasAccess('list'))
<a href="{{ url($crud->route) }}"><i class="fa fa-angle-double-left"></i> {{ trans('backpack::crud.back_to_all') }} <span>{{ $crud->entity_name_plural }}</span></a><br><br>
<a href="{{ url($crud->route) }}"><i class="fa fa-angle-double-left"></i> {{ trans('backpack::crud.back_to_all') }} <span class="text-lowercase">{{ $crud->entity_name_plural }}</span></a><br><br>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

text-lowercase?

@endif

<!-- Default box -->
<div class="box">
<div class="box-header with-border">
<h3 class="box-title">
{{ trans('backpack::crud.preview') }}
<span>{{ $crud->entity_name }}</span>
<span class="text-lowercase">{{ $crud->entity_name }}</span>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

text-lowercase?

<td>
<strong>{{ $column['label'] }}</strong>
</td>
@if (!isset($column['type']))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indenting below is odd.

@endforeach
@if ($crud->buttons->where('stack', 'line')->count())
<tr>
<td><strong>Actions</td>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You sure that doesn't have a translation key?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! Thank you.

@Laravel-Backpack Laravel-Backpack deleted a comment from lloy0076 Sep 28, 2017
@Laravel-Backpack Laravel-Backpack deleted a comment from lloy0076 Sep 28, 2017
@Laravel-Backpack Laravel-Backpack deleted a comment from lloy0076 Sep 28, 2017
@Laravel-Backpack Laravel-Backpack deleted a comment from lloy0076 Sep 28, 2017
@Laravel-Backpack Laravel-Backpack deleted a comment from lloy0076 Sep 28, 2017
@tabacitu tabacitu changed the base branch from master to dev September 28, 2017 06:09
@Laravel-Backpack Laravel-Backpack deleted a comment from lloy0076 Sep 28, 2017
@Laravel-Backpack Laravel-Backpack deleted a comment from lloy0076 Sep 28, 2017
@Laravel-Backpack Laravel-Backpack deleted a comment from lloy0076 Sep 28, 2017
@tabacitu tabacitu merged commit 222181a into Laravel-Backpack:dev Sep 28, 2017
@tabacitu
Copy link
Member

@AbbyJanke and @lloy0076 - thanks a lot for this. I've just merged it into dev.

@AbbyJanke very good work here. I've made just a few changes - mainly changed how autoSet() works - instead of creating the columns/fields no matter what, it only creates them if they're not already set. This allows us to use the "preview" functionality for non-autoset CRUDs too, and columns that have been defined by the user will look like he said, columns that he hasn't will look autoset. In most cases, this means the relationship columns won't be stripped out anymore. They'll just be pretty :-)

Again, thanks a lot for this, glad I've finally been able to merge this.

Cheers! Thanks for using Backpack.

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

Successfully merging this pull request may close these issues.

3 participants