Small changes to templates (form/centered layout) #750
Conversation
…age outside of the template Add support for hints in checkboxes in forms
Codecov Report
@@ Coverage Diff @@
## develop #750 +/- ##
=============================================
+ Coverage 66.05% 66.08% +0.02%
- Complexity 2223 2225 +2
=============================================
Files 118 118
Lines 5235 5239 +4
=============================================
+ Hits 3458 3462 +4
Misses 1777 1777
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## develop #750 +/- ##
=============================================
+ Coverage 66.05% 66.08% +0.02%
- Complexity 2223 2225 +2
=============================================
Files 118 118
Lines 5235 5239 +4
=============================================
+ Hits 3458 3462 +4
Misses 1777 1777
Continue to review full report at Codecov.
|
src/Layout/Centered.php
Outdated
| @@ -15,6 +15,10 @@ class Centered extends Generic | |||
|
|
|||
| public $defaultTemplate = 'layout/centered.html'; | |||
|
|
|||
| // Default atk4 logo | |||
| public $image = 'https://github.com/atk4/ui/raw/07208a0af84109f0d6e3553e242720d8aeedb784/public/logo.png'; | |||
There was a problem hiding this comment.
the $app class has a CDN string, which you can use for absolute perfection. The rest seems OK.
There was a problem hiding this comment.
Looks good.
But please address @romaninsh comment. Move logo URL to App->cdn['atk-logo']
|
@DarkSide666 I've added the requested changes, also quickly tested them on my install, everything seems to work as expected |
src/Layout/Centered.php
Outdated
| public $image_alt = 'Logo'; | ||
|
|
||
| public function init() | ||
| { | ||
| parent::init(); | ||
|
|
||
| // If image is still unset load it when layout is initialized from the App | ||
| if ($this->image === null && $this->app && $this->app->cdn['atk-logo']) { | ||
| $this->image = $this->app->cdn['atk-logo']; |
There was a problem hiding this comment.
This->app->cdn[‘atk’].’logo.gif’
There was a problem hiding this comment.
Dark no need to duplicate atk
There was a problem hiding this comment.
Oh right https://cdn.jsdelivr.net/gh/atk4/ui@1.7.1/public/logo.png will work too. I didn't thought about that.
Then this should simply be:
- remove atk-logo from App->cdn
- and here do like this:
if ($this->image === null && $this->app) {
$this->image = $this->app->cdn['atk'] . '/logo.png';
}
@pkly can you do these changes for us please so we can merge this in?
There was a problem hiding this comment.
This way is rather bad though.
For example, right now I can use the code that's implemented to just add my image directly to the App and forget about it.
The way you proposed it will never work, unless my image is named "logo.png", which is probably not ideal.
Actually, I'll do something slightly different in a little bit and let you know
There was a problem hiding this comment.
@romaninsh and @DarkSide666 how about this solution? I think it's the best of both worlds 35ab1b4
There was a problem hiding this comment.
I've also removed the logo url completely from App later, but forgot in that commit, but you can look over the next changes obviously.
|
And yes we do overuse cdn in project quite frequently. I wonder if there is a doc for that. |
|
Well, it looks good this way I guess. @romaninsh how you think? |
Fixes: #749
Adds support for custom icons in headers inside of the Centered layout.
Adds two additional tags for inserting html without re-writing the whole header section in the Centered layout.