Add option to display company names to labels #4405
Conversation
Add company name if it is turned on in settings and asset has a valid company
| @@ -118,6 +118,11 @@ | |||
| <br> | |||
| @endif | |||
| </div> | |||
| <div class="pull-left"> | |||
| @if (($settings->labels_display_company_name=='1') && ($asset->company_id!='0') && (!(is_null($asset->company_id))) && (\App\Models\Company::find($asset->company_id)->name!='')) | |||
There was a problem hiding this comment.
Could we maybe just do something like:
@if (($settings->labels_display_company_name=='1') && ($asset->company))
?
If the company isn't valid or if the asset doesn't have one, that should (I'd think) all be covered there.
| @@ -179,6 +179,10 @@ | |||
| {{ Form::checkbox('labels_display_tag', '1', Input::old('labels_display_tag', $setting->labels_display_tag),array('class' => 'minimal')) }} | |||
| {{ trans('admin/hardware/form.tag') }} | |||
| </label> | |||
| <label> | |||
| {{ Form::checkbox('labels_display_company_name', '1', Input::old('labels_display_company_name', $setting->labels_display_company_name),array('class' => 'minimal')) }} | |||
There was a problem hiding this comment.
This is nitpicky, but can you use the square brace array notation, just to keep it consistent with other blades?
{{ Form::checkbox('labels_display_company_name', '1', Input::old('labels_display_company_name', $setting->labels_display_company_name),['class' => 'minimal']) }}
| @@ -11,6 +11,7 @@ | |||
| 'checkout_date' => 'Checkout Date', | |||
| 'checkin_date' => 'Checkin Date', | |||
| 'checkout_to' => 'Checkout to', | |||
| 'company_name' => 'Company Name', | |||
There was a problem hiding this comment.
No need to create a new string here (that all those poor bastards have to translate). We have "Company Name" in admin/companies/table.name
| $setting->labels_display_company_name = 1; | ||
| } else { | ||
| $setting->labels_display_company_name = 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
A few small (again, nitpicky) things here:
-
We've been switching over to the more modern
$request->input('foo')(or in this case,$request->has('foo')) notation (instead of the olderInput::get()facade) for request data (and this method already passes it as part of the signature) -
For something small like this, I usually like to keep the annotation simpler, like:
$setting->labels_display_company_name = $request->input('labels_display_company_name', '0');
We don't really need all the if/else business, we just need to tell the code to take the value if there is one submitted, and if not set the value to 0.
| public function up() | ||
| { | ||
| Schema::table('settings', function (Blueprint $table) { | ||
| $table->tinyInteger('labels_display_company_name')->default(0); |
There was a problem hiding this comment.
Probably better to make this is boolean?
$table->boolean('labels_display_company_name')->default(0);
|
I think I have covered all changes, plus I made the square brace change on all the |
| @if (($settings->labels_display_company_name=='1') && ($asset->company)) | ||
| C: {{ $asset->company->name }} | ||
| @endif | ||
| </div> |
There was a problem hiding this comment.
One more thing - do we want the empty divs if company support isn't enabled on labels? Maybe:
@if (($settings->labels_display_company_name=='1') && ($asset->company))
<div class="pull-left">
C: {{ $asset->company->name }}
</div>
@endif
I'm honestly not sure - the labels are pretty cramped anyway, so I'm guessing we wouldn't want it, but def open to discussion.
|
@snipe all right all changed |
|
Nice, thanks! |
Feature Request #4374
This PR will add an option in the labels settings page to show the company name on the asset labels when they are generated. The company name will not shown if the asset has the company_id set to NULL or 0 even if the option is enabled.
As always open to feedback and any changes needed.