Conversation
|
@mattermoran you are the most qualified to review this code.
|
|
Good job, @Spazcool . Definitely looks better than before. Here's a little 'sketch' of what I think is not good. Not changing design or anything just tiny fixes that would make the page look better. Especially paddings should the same everywhere. |
|
Now with more consistent padding! I tried without background colors, it bothered me to my very core. |
|
|
|
|
@lenzai @mattermoran I believe I've tackled all the issues you listed above. I'm not sure I like the way the breadcrumbs look with the extra padding, especially on the search page, but I defer to your guys' expertise. Padding between similar items: Wrench & search page: |
|
how about something like that for wrench and picture? |
|
@lenzai your posted img doesn't look all that different from the old style, minus the margins. Do you want the img to be big or small?
|
|
Picture roughly same size as the container box. Text for title on one line |
|
It looks cleaner. Side by side your img is nicer, but again its a small img that will be viewed on a mobile. Do we want big imgs or streamlined item cards? |
|
My attempts at incorporating Alex's style.
|
|
I want the image to be roughly the same size as the container. What you call the "old style" is an image that barely use half the the height of the container. Seems like extremely inefficient use the screen real estate. Once this design/coding issue is solved, we can afford to make the container fit roughly the same height as the text because we have confidence that we won't have 10px height picture as consequence. I hope Alex can guide you through best practices to achieve that with minimalistic code. |
|
Without the background color:
|
|
Some kind of background for title area is fine. |
lenzai
left a comment
lenzai
left a comment
There was a problem hiding this comment.
so we got rid of bootstrap and replace with many new classes.
a few div nesting disappeared.. Great!
But what else do we get in term of code readability and maintainability?
| @@ -1 +0,0 @@ | |||
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 23 23"><path d="M1 1h7v7h-7zM11 1h2v2h1v1h-1v1h1v6h-1v-2h-1v-1h1v-2h-1v-3h-1v3h-1v3h-1v-6h1v-1h1zM15 1h7v7h-7zM2 2v5h5v-5zM16 2v5h5v-5zM3 3h3v3h-3zM17 3h3v3h-3zM11 7h1v1h-1zM1 9h1v1h-1zM3 9h5v1h-2v1h3v-1h1v-1h2v2h-2v1h-5v-2h-1v1h-1zM15 9h5v3h1v1h-3v-1h1v-2h-4zM21 10h1v1h-1zM1 11h1v1h-1zM12 11h1v1h-1zM14 11h1v3h-1zM3 12h2v2h-4v-1h2zM16 12h1v2h2v1h-2v1h-2v-1h1zM6 13h3v1h-3zM10 13h3v1h1v5h-1v-2h-2v-1h2v-1h-1v-1h-1v2h-1v-1h-1v-1h1zM21 13h1v1h-1zM1 15h7v7h-7zM19 15h2v1h-1v1h-2v-1h1zM2 16v5h5v-5zM9 16h1v3h1v1h-2zM21 16h1v2h-1zM3 17h3v3h-3zM15 17h1v1h-1zM11 18h1v1h-1zM16 18h3v2h1v1h-1v1h-1v-1h-2v-1h2v-1h-2zM12 19h1v1h-1zM14 19h1v1h-1zM9 21h2v1h-2zM12 21h1v1h-1zM20 21h1v1h-1z"/></svg> No newline at end of file | |||
| <span> <a href="/search?floor=<%- floor %>"> <%- floor.toLowerCase() %> </a></span> > | ||
| <span> <a href="/search?floor=<%- floor %>&room=<%- room %>"> <%- room.toLowerCase() %> </a></span> > | ||
| <span> <a href="/search?floor=<%- floor %>&room=<%- room %>&location=<%- location %>"> <%- location.toLowerCase() %> </a></span> > | ||
| <span> <a href="/search?fixture=<%- fixture %>"> <%- fixture.toLowerCase() %> </a></span> |
| @@ -1,7 +1,7 @@ | |||
| <% include ./partials/header %> | |||
| <div class="resultContainer col-xs-12"> | |||
| <h1>Recently scanned QR code</h1> | |||
There was a problem hiding this comment.
why so picky about custom styling?
| <% include ./partials/header %> | ||
| <div class="resultContainer col-xs-12"> | ||
| <h1>Recently scanned QR code</h1> | ||
| <article class="col-xs-12"> |
|
@mattermoran is this what you recommend in term of optimal CSS/class coding? |
lenzai
left a comment
lenzai
left a comment
There was a problem hiding this comment.
Let @mattermoran review CSS/HTML code
3 participants
Removed forced upper & lower casing, nesting, margin and padding. Increased img size. Added link to img. #48 #39 #19
NEW STYLE:
OLD STYLE:
