Add-on with long name is causing layout issues #2465
Conversation
tsl143
force-pushed
the
I2415_2
branch
4 times, most recently
from
3a8cecd to
3b6089c
Compare
src/core/css/SearchResult.scss
Outdated
| margin: 0; | ||
| padding: 0; | ||
| text-decoration: none; | ||
| width: calc(100% - 40px); |
There was a problem hiding this comment.
To aid us in the future can you add comments for what the calc is based on in both cases?
There was a problem hiding this comment.
For addon listing
Calc(100% - 40px) - 38 px is fixed width .SearchResult-icon hence (100%-40px) for .SearchResult-heading
For theme listing
Calc(100% - 85px) - 85 px is fixed width .SearchResult-rating hence (100%-85px) for .SearchResult-heading
There was a problem hiding this comment.
@tsl143 thanks, could you add those details to the CSS as comments please.
|
@tsl143 thanks for the patch. Please can you add before and after screenshots to the pr? |
|
@muffinresearch Screenshots FOR ADDONS LISTINGBefore After FOR THEMES LISTINGBefore After |
|
Is there a way to do this with flexbox instead of all the floats and calc? I appreciate the floats were there before but I think this might be cleaner with flexbox directly. |
|
There could do with being more whitespace between the text + ratings, so I'd allow for at least 10px gutter between them so things don't look too cramped. |
|
We can for sure use flexbox (always like them ) |
Yep that should be fine. |
|
@tsl143 hi, hope you had a good vacation. Let me know if you have any updates to review. |
|
Thanks, yes indeed I had a really good one :) |
|
@tsl143 you can configure the api host with a local config see https://github.com/mozilla/addons-frontend#configuring-for-local-development for details. This example should work for -dev: // config/local-development.js
module.exports = {
apiHost: 'https://addons-dev.allizom.org',
amoCDN: 'https://addons-dev-cdn.allizom.org',
}; |
|
This is what I was looking for, thanks :) |
|
@tsl143 feel free to just update this PR. |
|
Screenshot after
|
src/core/css/SearchResult.scss
Outdated
| width: 38px; | ||
|
|
||
| [dir=rtl] & { | ||
| margin: 0 5px 0 0; |
There was a problem hiding this comment.
We have a mixin for this in core/css/inc/mixins: @include margin-start(5px);
There was a problem hiding this comment.
(It would go in the main selector though, where margin-left is.
src/core/css/SearchResult.scss
Outdated
| font-size: $font-size-default; | ||
| font-weight: bold; | ||
| margin: 0; | ||
| min-width: 1%; |
There was a problem hiding this comment.
The flex-item is overflowing so to control that.
There was a problem hiding this comment.
|
ping @tofumatt , @muffinresearch |
|
The issue has already been fixed by layout change, hence closing |
FIX mozilla/addons#10391