Conversation
|
Hey @johnkeough I see you’re still working on the styles. I only have a heads up: It would be great if you could exclude all the files that are built automatically from your pull requests, like all the files in the build or docs folder. It’s easier to look at the changes without these files. Tell me if you need help doing that. Looking forward to see your work! :) |
|
Hey @gchtr ! Absolutely. I haven't before though, would you mind giving me a few pointers on how to do that? Thank you! :) |
|
Okay, let’s see. What do you use to manage Git? An app like Sourcetree, Fork or GitHub Desktop or do you use Git with the CLI? |
|
I use Git with the CLI for mostly all, no external apps. |
gchtr
left a comment
There was a problem hiding this comment.
Wow, that looks great! I’ve always been in love with the GT America font. And I love what you did with the syntax higlighted parts! I’ve yet to get used to all the dark modes and interfaces. I saw that it’s based on the new Upstatement design. Nice!
I only have a couple of points where I think we have to find a compromise between design decisions and the reality that docs are a daily tool for developers that needs to readable and easy to navigate.
I saw that you only added light font styles. I guess now all strong text is rendered with a faux bold style, which is generated by the browser. Do you have a bold or even an extra bold font that you could add? I know, it’s a design decision to not use bold. But in the Timber documentation, we kinda use the convention to use <strong> to mark file names and file paths.
To me, the problem can also be seen in the navigation. Here, I think it’s too hard to see on which page I currently am:
Currently, there’s no way to see where there’s a link. Take this example:
There are actually two links in here:
It’s important to not only style text links, but also <code> elements that are links.
I realized it might not even be enough to style links with color only, as it’s done in the current documentation. We should probably add underlines to links in content, because we already use colored text a lot in the content when using <code> elements. That would also be better for acessibility, especially for people with a form of color blindness.
It’s a different thing with navigations or the table of contents, which consist mostly only of links. But then there are elements like tables with an overview over object properties and methods, where sometimes there are links, and sometimes there aren’t.
It would be great if you could improve on these points :).
|
@gchtr thanks for the helpful feedback as always. @johnkeough got called away this week for another project, but when he returns I'll help him address these points in the next go! |
|
@johnkeough let's talk next week about a solution to the font issue. We can serve crossdomain fonts from upstatement.com, but I'm not sure it's worth the effort. FWIW I think it looks glorious in Helvetica. |
|
Thank you for the great feedback @gchtr ! Here are the updated changes @jarednova :
|
|
Looking much closer! I think everything you flagged in the last pass @gchtr has been taken care of. There's one more area I'd like to spend a little more time. These tables with the method/property type and definitions: I was looking to apply some surrounding spans for |
gchtr
left a comment
There was a problem hiding this comment.
This is looking good now! I have only some changes in #30, but then this is good to go.
@jarednova I added some spans and classes to the Teak markup in the 1.0.3 release. You can get it with composer update timber/teak in the docs repo. Will that work?
|
@gchtr I think so! I applied a few last color/style tweaks. The last fix depends on a PR I just submitted to Teak here: ... this will allow proper lines breaks in the MD files which trigger the correct markup from 11ty. Could you give this one last look before we merge? I want to make sure the changes I made in 1d703ba work (I needed to include |
Should be fixed now.
I had a look and to me, all is fine now 👍
I forgot to remove an entry in webpack.mix.js, that’s why that error appeared. Fixed in @jarednova I think you can go ahead and merge this. |
|
¡¡¡¡MOST EXCELLENT!!!! |
Uh oh!
There was an error while loading. Please reload this page.