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

NEW THEME: Kinda Dark #78

Merged
merged 3 commits into from Apr 6, 2019

Conversation

Projects
None yet
2 participants
@sammdu
Copy link
Contributor

commented Apr 6, 2019

I created this theme by editing the "Breathe" theme included in Customizer.
Darkened a few spots to a dark-bluish gray, and changed the the accent color to a vibrant blue.
This is the theme that I use for my own instance of Kanboard.

sammdu added some commits Apr 6, 2019

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 6, 2019

Screens (also included in the edited README of the themes folder)

Board View
Dash View

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 6, 2019

I wonder if it is possible to change the HTML of the page as well, because the default avatar (profile picture) icon's resolution is way too low.

<div class="avatar avatar-20 avatar-inline">
<img src="/kanboard/?controller=AvatarFileController&amp;action=image&amp;user_id=1&amp;size=20" alt="Samm Du" title="Samm Du">
</div>

If you look at the end of the request URL, size=20 is asking for a 20x20 pixel icon, which is way too blurry.
I have found (by messing with the developer tools in my browser) that 50x50 is a good resolution that it's clear, but won't slow down the page.
It'll be great if that's possible to change.

Maybe even include that (the ability to change the icon size and resolution of the avatar) as an option in the Customizer settings page among the image assets and login page stuff!

@creecros

This comment has been minimized.

Copy link
Owner

commented Apr 6, 2019

I'll merge and take a look at the avatar next time I open my laptop, pretty sure it runs through a controller, since I had to add a custom size in group_assign...

@creecros

This comment has been minimized.

Copy link
Owner

commented Apr 6, 2019

@sammdu sammdu changed the title Kinda dark NEW THEME: Kinda Dark Apr 6, 2019

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 6, 2019

@creecros thanks very much! Appreciate your work!

@creecros creecros merged commit 3918592 into creecros:master Apr 6, 2019

@creecros creecros referenced this pull request Apr 6, 2019

Merged

Update Plugins #156

@creecros

This comment has been minimized.

Copy link
Owner

commented Apr 8, 2019

@sammdu
to adjust the avatar size, would require an override not currently being made, for:
https://github.com/kanboard/kanboard/blob/master/app/Template/header/user_dropdown.php

would also require extending the controller, unless the only other size you offer is 48px.

would probably take some work to make a slider with a live preview, and some thought to figure out the correct css for each size you offer in the slider, as well as getting the carrot-down back to the center
image
and I couldn't figure out a simple calculation to get that correct for each size...at 30px padding-top needs to be 7px with a vertial-align to top(why the hell vertical-align:middle doesn't work, is beyond me), at 50px it needs to be 15.5px, 14.5 at 48px...and that was eye balling it. and you would have to store that value into the config and have a solve for each possible size, so that you can pull that value out and place into the element style based on selected option.

tbh, not sure I feel like doing all that...at least not today.

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

@creecros
Yeah that does seem like quite some labor just to adjust an icon. It's okay though, we can take our time.
We can settle on a standard size if necessary, if 48 is it, then 48 it is.
As for the aligning, (even though it may seem like an overkill), I'm thinking of just slapping a flexbox on there - that's how I do all my centering nowadays. It's just too convenient.

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

image

Here, as you can see, flexbox works!

And we can change the border-radius of the avatar to 50% so it stays circle regardless of image size (but I kind of like the rounded corner square look that I'm getting in the above pic idk...to keep it that way we can set the border-radius to 25%)

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

Or we can simply do away with the triangle all-together like Google does.
I don't see why that can't work.
Then all we need to modify is the avatar itself.
But that's a theming choice though, probably a good idea to leave that option available for other themes.

@creecros

This comment has been minimized.

Copy link
Owner

commented Apr 8, 2019

@sammdu
actually, 48px isn't as built in as i thought
https://github.com/kanboard/kanboard/blob/0b4b288ff8b2a9e6f9fad74b29123e5b8daa6089/app/Helper/AvatarHelper.php#L63-L67

it will still require extending the helper(i keep saying controller when I mean helper), to add a 48px option. Or could bypass it entirely and store all the css options into the config...

I don't know wth a flexbox is, i'm not the css guy, @kenlog is better at that than myself.

@creecros

This comment has been minimized.

Copy link
Owner

commented Apr 8, 2019

and tbh, if allowing it to go square and not stay a circle is ok, then it should be fairly simple with out all the BS.
image

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

@creecros
You know php stuff the best - you can definitely make that call.
I can help with the css as well. I already have a "demo" (sort of) running in developer tools already using flexbox - and if we're going with that - all we need to do is copy down that code.

I would still override the default css though, because the original code is defined using absolute values - pixels, which can cause differences across different screen resolutions and so forth.
I would still write the border-radius in terms of percentage, so that it stays consistent across devices.
That's just a matter of modern UX best practice. Responsive design and all that good stuff.

Again, whether the logo shows one one way or another I think should be a theming call, and we should leave them as they are, and allow themes to change them to their taste. I can easily have rounded corners in my theme, but I think it should also be possible for other themes to have a circle if they want.

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

better yet - (and I don't mean to keep adding on to your workload) have that as another option in customizer, to switch between Square border-radius: 0;, Rounded Square border-radius: 20%;, Circle border-radius: 50%;, and just swap the css per user selection.
Again, we don't have to worry about this getting done right away, but something to think about for the future.

@creecros

This comment has been minimized.

Copy link
Owner

commented Apr 8, 2019

ya, next chance i get.

i'm thinking more along the lines of putting it under the theme section though, feel like it fits there better than image assets & settings

image

@creecros

This comment has been minimized.

Copy link
Owner

commented Apr 8, 2019

or not, hell i don't know...

@creecros

This comment has been minimized.

Copy link
Owner

commented Apr 8, 2019

Got about 90% done, just need to finish the javascript, modify the new helper a bit, and then I can test.

I'll wrap it up tonight most likely. ..

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

@creecros Great! Let me know if you need any help front-end wise.

@creecros

This comment has been minimized.

Copy link
Owner

commented Apr 9, 2019

@sammdu

all set, on the avatar branch. i'll let you take a look and see if you have any edits you'd like on the front end, before i merge and update a release.

couple notes:
1.) what a pain in the ass that was (I waisted a lot of time in js, which i suck at by the way, but figured out the conditional statement for the queryselectors had to be there, apparently, if the first is null, it doesn't do the second)
2.) the live preview, uses the current users avatar, if that's an image...it will have to render each step in the slider, so it will lag the first swipe through while it renders and adds each pixel size to the cache. i could step it by 5 pixels to reduce this, and i only set to allow 20 to 50 pixels.
3.) it uses the cache, so if an avatar image is changed and say pixel 33 was never rendered, then you will have mixed images in the slide, this isn't likely to happen though, but clearing cache will fix.

The rabbit hole of using a render without cache, or even adding an id to the img element is a nightmare, i had to grab the img src in js and truncate the last 2 characters and then rewrite the src adding the slider value...

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

@creecros Looks great to me!
It doesn't lag on my end, though the test server is on a local VM so there isn't the network in the way.
(p.s. when I have to write JS I just copy-paste from StackOverflow then fiddle with the logic. So ya, I feel you.)

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

Oh and, if it's not too late, maybe have the radius slider go all the way down to 0 so that there's an option to have a square avatar.

@creecros

This comment has been minimized.

Copy link
Owner

commented Apr 9, 2019

Now goes to zero.

I'll merge and update release a bit later

@sammdu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

@creecros
Good work man. Appreciate your effort!

@creecros

This comment has been minimized.

Copy link
Owner

commented Apr 9, 2019

Thanks, no worries, it's how I learn things, like flex boxes...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.