-
Notifications
You must be signed in to change notification settings - Fork 0
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
Rebrand BODS site to match OO styles #57
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@Lathrisk - I think this is looking great. From these screenshots I'd say you're pretty much done :) On the specific points:
The tba was just about the location really - I was thinking in the left sidebar on desktop, perhaps above the title on mobile? On the register we add a small tagline to the logo: Something similar might also work here?
I don't think we should abbreviate here, is it difficult to push the title onto two lines like:
If so, losing the logo is probably preferable, especially if we get it in on the homepage anyway.
Yep, happy to review this in more depth tomorrow, but as-per above I think we're nearly there. Obviously if you feel there's more work to do, please flag! Is it possible to get a staging site set up so that we can get non-technical members of the team to review too? |
The demo site is now available at - http://standard.openownership.org/en/56-theme-rebranding/index.html |
Thanks @Lathrisk: theme is looking really good. My only concern is about navigation back to the main Open Ownership website. I think we do need some way for people to find their way back... but I'm not sure what that is right now. @stevenday may have an idea, but if you can think of any good options to suggest -that's welcome too. |
@timgdavies at present the Open Ownership logos in the sidebar on desktop and burger nav dropdown on mobile provide a link back to the main OO page. I can have a think about making that more explicit though. |
Ah-ha - I needed to do a hard refresh. Forgot that RTD seems to need that at times. Ok - that works for me for now :) I'll leave @stevenday to do the proper review - but I'm happy :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lathrisk - this is looking great. A few minor things from me that I think will finish it off with regards to consistency, but feel free to point out if they won't work for any reason:
- Can we use the blue on black version of the icon in the nav? Everything else uses this (admittedly as part of the larger logo) and I think it's almost more recognisable in blue.
- Can we reduce the font-size of the menu to
0.9rem
, and useAtlas Grotesk Bold
instead of the regular version of that font? I think with this, we might get away with keeping the small logo in there on mobile too. - Can we drop down the
h1
size to2.25rem
? - Can we use the white-on-black version of the logo in the homepage sidebar, and make the version number background black too?
h2
's on the main site are2rem
(not1.8rem
) and have a weight of400
, not500
- Normal body text is
1rem
, not1.15rem
(this seems to come from hypothesis, so not sure if you can just take it out or if you have to override it). Likewise, our normal line height for paragraphs is1.6
, not1.7
.
A more speculative question, I wonder if it's possible to do a quick and dirty rebrand of the SVG's by just swapping #00deb6
in all of them with #3c31d4
?
Thanks, @stevenday, I'll take a look at these changes soon. They all look reasonable and shouldn't take long. Editing the SVG like you suggested is actually how I changed it from blue to white in the first place. I can't remember what my reason for that was, possibly contrast, but it is a deviation from the Open Ownership website. |
@stevenday is this the kind of layout you were thinking of? (EDIT: all these changes are on the demo site now - http://standard.openownership.org/en/56-theme-rebranding/index.html) Would you prefer the OO logo above or below the version number in the sidebar? Conscious that the blue logo repeats quite close together. And this for the SVGs? |
@Lathrisk - yes, I think that's fine in the first screenshot - under the version number. Yesterday, @timgdavies had the idea that we could also add an explicit text link under the OO logo which reads "Back to main site", (in the same font/size etc as the nav links) would it be ok to add that in?
They all look good to me except this: Which is at the bottom of this page: http://standard.openownership.org/en/56-theme-rebranding/primer/whatisthebods.html Just to double check, have you applied the same change to the Russian language SVGs? (I don't seem to be able to switch to Russian on the branch preview). |
@stevenday ah, I missed that one (json). I think they're all slightly different variants of the same colour because it was a string replacement. And yes, adding a little text under the logo shouldn't be a problem. I'll group the logo and text into a single link. I did update the Russian SVGs as well but I've not tested them. I've not looked at the translation stuff much yet but will do some investigation. |
@stevenday @timgdavies I've added the 'Back to main site' tagline - http://standard.openownership.org/en/56-theme-rebranding/ (P.S. There seem to be some github issues, so things haven't been updating quite right) |
Thanks @Lathrisk! I'm really nit-picking now, but could we have a bit more space between the logo and the link? A bottom margin on the image seems to do the trick: I also added a sans-bold class, made the text white and |
@stevenday I've added those latest changes to the test site. I'll be on other projects until Tuesday now, but I can pick any additional changes/feedback on Tuesday morning. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lathrisk - a couple of minor comments inline, otherwise I think we're pretty much there. Thanks very much for all the work on this, it's looking great 👍
I've realised I was slightly wrong about the h1
- on the main site we make them 3rem
on screens wider than 1440px, 2.25rem
between 1440px - 640px and 2rem
below that.
Somehow, it also looks like we're getting a font-weight: bold
, rather than font-weight: 400
just on H1
s - it's coming from oods.css.
Otherwise I've tested on mobile, in chrome, FF and safari and it's all looking good!
Thanks @stevenday I'll take a look at these next week. We can definitely drop some fonts, if page weight is a concern then we should also think about minifying assets as part of the build pipeline. That was one on my tech debt shortlist. I think I only have 1 day for BODS work next week in this block, but should be able to rattle through a fair bit of this. |
I'm not overly concerned at the moment, I just thought it wasn't worth adding anything new unnecessarily. That said, I think this is a good thing to add a ticket for, so I've made a placeholder: #60 - would you mind filling that out with some basic ideas of what you'd like to do @Lathrisk? |
50854d1
to
cb92188
Compare
…as extraneous classnames
ce2d851
to
6501090
Compare
I've rebased this over master and squashed my fixup at the same time. |
#56
Read the Docs demo page:
http://standard.openownership.org/en/56-theme-rebranding/index.html