Conversation
|
Deployment previews on netlify for branch
|
Bug: T422593
outdooracorn
left a comment
outdooracorn
left a comment
There was a problem hiding this comment.
Managed to do a quick review.
I would comment out all the custom CSS and see how far you can get with Vuetify-native CSS classes. Reach out for collaboration if that is proving difficult. If we have to add custom CSS we should also write a comment explaining why it was necessary.
outdooracorn
left a comment
outdooracorn
left a comment
There was a problem hiding this comment.
Another quick review with some nitpicks. We are getting there :D
| <h3 id="target-audience">1.1 Target Audience</h3> | ||
| <ul> | ||
| <h3 class="mb-3 mt-6" id="target-audience">1.1 Target Audience</h3> | ||
| <ul class="pl-5 mb-3"> |
There was a problem hiding this comment.
Vuetify2 has a padding-left: 24px on ul elements by default (equivalent to a pl-6 class). So pl-5 will reduce the padding by default. Was this intentional? I'm inclined to just use the defaults.
Let's also use mb-4 to match the bottom padding of p elements.
| <ul class="pl-5 mb-3"> | |
| <ul class="mb-4"> |
Let's do the same for the other ul elements too (except for lists nested in other lists).
There was a problem hiding this comment.
I wanted the design to remain close enough to the current one. So I tried to stay with the original style where possible. The css I deleted reduced the default ul padding
There was a problem hiding this comment.
I will do as you suggested
There was a problem hiding this comment.
Thanks, I think we weren't super clear about whether this change had to match the existing CSS exactly. Sorry about that!
You are also right in that a refactor shouldn't change the functionality.
| <li><strong>Local policies</strong> set by individual communities for their Cloud instances are out of scope for this document; they can operate independently, but within the boundaries defined here.</li> | ||
| <li>In a broader context of the Wikibase ecosystem, it also complements:</li> | ||
| <ul> | ||
| <ul class="mt-2 mb-2"> |
There was a problem hiding this comment.
mb-2doesn't have an effect because of the margins on the parent list.- I don't think we want
mt-2in this nested list.
| <ul class="mt-2 mb-2"> | |
| <ul> |
2 participants
Bug: T422593