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

Added white to avialable appearance colours #789

Closed
wants to merge 12 commits into from

Conversation

Projects
None yet
3 participants
@nophead
Copy link
Contributor

commented Feb 27, 2015

One of my machines is white Dibond. Not sure what to do about the acrylic one!

@thinkl33t

This comment has been minimized.

Copy link
Contributor

commented Feb 27, 2015

@nophead how about this:
generic_transparent_d

@nophead

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2015

Yes that would be nice but is it available as a colour in HTML?

On 27 February 2015 at 13:48, Bob Clough notifications@github.com wrote:

@nophead https://github.com/nophead how about this:
[image: generic_transparent_d]
https://cloud.githubusercontent.com/assets/1016780/6413423/38c3f320-be87-11e4-9902-4ce6c9ef9651.jpg

Reply to this email directly or view it on GitHub
#789 (comment).

@thinkl33t

This comment has been minimized.

Copy link
Contributor

commented Feb 27, 2015

@nophead

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2015

Very nice. Can you explain what the deal is with octoprint.css please. What
is is compiled from and with?

On 27 February 2015 at 17:42, Bob Clough notifications@github.com wrote:

Yep, check it out:
[image: screenshot from 2015-02-27 17 40 01]
https://cloud.githubusercontent.com/assets/1016780/6417775/c4c31d9a-bea7-11e4-9afb-8a30fa4fc1c1.png

code @ https://github.com/thinkl33t/OctoPrint/tree/devel

Reply to this email directly or view it on GitHub
#789 (comment).

@thinkl33t

This comment has been minimized.

Copy link
Contributor

commented Feb 27, 2015

octoprint.css is compiled from octoprint.less. less is a css preprocessor that lets you use nice things like variables and functions to make pretty cross platform css.

You can compile it under linux using lessc:

  • apt-get install node-less
  • lessc -x octoprint.less > ../css/octoprint.css
@nophead

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2015

So this PR has a bug because I don't understand css. I assumed that the colour string was just used as an HTML colour but it seems it is a css class name? I think it appeared to work for me because the default class sets the colour to very slightly off white.

@thinkl33t 's version is better so I will close this. His is missing the image though so I have added that to my fork.

@nophead nophead closed this Feb 28, 2015

@nophead

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2015

I clearly don't understand git either. Does this PR include commits to my fork that I made after I made the PR? In which case I could reopen it. Or do I have to submit a new one?

Will it get merged anyway as the transparent effect is a bit obscure?

@foosel

This comment has been minimized.

Copy link
Owner

commented Feb 28, 2015

Anything that you commit to the branch you created the PR against will be added to the PR. The advantage of this is that it makes stuff like reviewing a PR less of a hassle - the PR author can just fix things that are not perfect yet, commit the fixes to the PR branch and they become automatically part of the PR. this is one of the reasons why I usually only accept PRs against a special PR branch, not devel itself, people tend to accidentally clutter up the PR branches if they are not aware of that, create the PR against their version of devel and then after creating the PR add other changes.

Maybe some quick words on how that appearance color stuff works. Since it's not just a static background color but actually a gradient it depends on css classes named like the colors. So adding a simple class white to the correct location in octoprint.less is needed in order for this to work properly - selecting the value "white" for appearance will set the class of the navbar to white, so it would make sense to also declare a class of this name.

If you want you can re-open the PR, or I'll just quickly give your and @thinkl33t 's changes a manual test drive and merge them that way.

@nophead

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2015

Ok, thanks for the explanations.

@nophead nophead reopened this Feb 28, 2015

@nophead

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2015

I just noticed my version is lacking a gradient. I think it needs @thinkl33t's, version of the image as he must have put the gradient in the image.

@thinkl33t

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2015

I'll re-commit it shortly
On 28 Feb 2015 12:59, "Chris" notifications@github.com wrote:

I just noticed my version is lacking a gradient. I think it needs
@thinkl33t https://github.com/thinkl33t's, version of the image as he
must have put the gradient in the image.


Reply to this email directly or view it on GitHub
#789 (comment).

@nophead

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2015

Thanks Bob.

acrylic mendel90 octoprint - mozilla firefox 28022015 155127

@foosel

This comment has been minimized.

Copy link
Owner

commented Feb 28, 2015

Pure CSS :D

image

@foosel

This comment has been minimized.

Copy link
Owner

commented Feb 28, 2015

Merged, but without the gradient including image. Instead I made it based purely on a combination of the gradient less image and a CSS gradient overlayed on top of it. Has the nice added side effect that I could turn transparency itself into a toggleable option and you can now have all of the available colors semi-transparent - let's face it, there is not only crystal clear acrylic, and while we are being a bit silly, let's go all the way ;)

2015-02-28 23_39_56-devel kara octoprint

2015-02-28 23_40_35-devel kara octoprint

@foosel foosel closed this Feb 28, 2015

@nophead

This comment has been minimized.

Copy link
Contributor Author

commented Mar 1, 2015

Great thanks. Apologies for off topic questions but Git and Github have completely kettled my swede! The more I use them the less I understand.

What is the correct method to make my Devel branch catch up with yours? After wrestling with it for a day and having it tell me I was working with a detached head at one time, the best I have achieved is resetting to before I merged @thinkl33t's graded image and then merging your changes. That seems to work but it says I am one commit ahead and it has a different hash. Before these changes my devel was the same as yours with the same hash as it was freshly forked. I have the same issue with the local copy on the RPi unless it is a fresh clone.

Since this is shown on the page and used for bug reporting I assumed identical code would give an identical number.

@nophead

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2015

OK I worked it out so I can answer my own question:

I should have done all my work in a topic branch and not disturbed my copy of devel. That way I can merge your upstream commits as simple fast forwards and get the same hash. The other mistake I made was doing the merges in github. I should do them all locally with git and then just push the results to github.

To fix the mess I hard reset my devel to before I touched it and then pulled your upsteam changes, which of course contained the changes I just discarded with the reset.

Apologies for the noise.

@foosel

This comment has been minimized.

Copy link
Owner

commented Mar 2, 2015

@nophead I was about to explain this to you this morning, but apparently you successfully figured it out yourself :)

No problem regarding "the noise" either ;)

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.