-
Notifications
You must be signed in to change notification settings - Fork 9.2k
[IMP] accounting: cheat sheet styles #13047
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
Conversation
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.
The proposed change seems nice but the way we are doing it seems weird tho 😬
static/css/accounting.css
Outdated
@@ -11,15 +11,21 @@ main.index .toctree-wrapper > .row:first-child > .col-md-3 { | |||
.doc-aside p{ | |||
padding: .5rem; | |||
} | |||
.doc-aside div { | |||
border: 1px solid rgba(255, 255, 255, 0); |
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.
I'm not sure what is the point of this. This will put an invisible border (?) What are we trying to achieve ?
It happens a lot in the pr.
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.
Hi @rugo-odoo, the reason I added an 1px invisible border is to avoid the 1px layout shift that happens when the 1px black border appears on hover :)
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.
I believe much of this is unnecessary.
-
For the one with
.highlight-op
, we can just change the border color. We already have a border so it won't shift. -
If I'm not mistake, to prevent the shift, we can just use
box-sizing: border-box
for the elements we need. border-box will not add any width/height with the border.
It will be cleaner and easier to maintain.
@robodoo delegate=rugo-odoo |
static/css/accounting.css
Outdated
@@ -11,15 +11,21 @@ main.index .toctree-wrapper > .row:first-child > .col-md-3 { | |||
.doc-aside p{ | |||
padding: .5rem; | |||
} | |||
.doc-aside div { | |||
border: 1px solid rgba(255, 255, 255, 0); |
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.
I believe much of this is unnecessary.
-
For the one with
.highlight-op
, we can just change the border color. We already have a border so it won't shift. -
If I'm not mistake, to prevent the shift, we can just use
box-sizing: border-box
for the elements we need. border-box will not add any width/height with the border.
It will be cleaner and easier to maintain.
c71c50f
to
e92d9c0
Compare
Hi @rugo-odoo! Thank you for your helpful suggestions :) in e92d9c0 I was able to replace the border color for For your second point, about using With these changes, things should be cleaner and easier to maintain! Thank you for pushing for changes :) ready for another look! 🙏 |
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.
@robodoo r+
closes #13047 Signed-off-by: Ruben Gomes (rugo) <rugo@odoo.com>
Task: #4743756
FWP up to
master
This PR improves the styles on the Accounting Cheat Sheet by adding borders which adds more contrast to the highlights.
As well as corrects the color of the Reconciliation button to use the Odoo primary and secondary colors: