Skip to content
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

fix: Add more RTL improvements #526

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 8 additions & 30 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,36 +3,14 @@ name: Lint
on: [push]

jobs:
flake8:
name: flake8
ruff:
name: ruff
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
- name: Set up Python
uses: actions/setup-python@v2
with:
python-version: 3.9
- name: Install flake8
run: pip install --upgrade flake8
- name: Run flake8
uses: liskin/gh-problem-matcher-wrap@v1
with:
linters: flake8
run: flake8
- uses: actions/checkout@v4

- run: python -Im pip install --user ruff

- name: Run ruff on djangocms-admin-style
run: ruff --output-format=github djangocms_admin_style

isort:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
- name: Set up Python
uses: actions/setup-python@v2
with:
python-version: 3.9
- run: python -m pip install isort
- name: isort
uses: liskin/gh-problem-matcher-wrap@v1
with:
linters: isort
run: isort --check --diff djangocms_admin_style
49 changes: 31 additions & 18 deletions djangocms_admin_style/sass/components/_base.scss
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The icon for messagelist is hovering over the text

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

border-radius on lines 131 and 136
padding-left on line 201.
padding on line 329.

Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,18 @@
body {
background-color: $gray-lightest;
h1 {
margin-left: 0 !important;
margin-inline-start: 0 !important;
}
.module {
h2 {
padding-left: 0;
padding-inline-start: 0;
}
p {
padding-left: 0;
padding-right: 0;
margin-left: 0 !important;
padding-inline: 0;
margin-inline-start: 0 !important;
}
table {
margin-bottom: 20px !important;
margin-block-end: 20px !important;
}
}
}
Expand Down Expand Up @@ -74,7 +73,7 @@ h6 {

ul,
ol {
margin-left: 20px;
margin-inline-start: 20px;
}

// buttons and links
Expand All @@ -86,9 +85,15 @@ a.btn {
float: right;
}

[dir="rtl"] {
.button.default, input[type=submit].default, .submit-row input.default {
float: right;
fsbraun marked this conversation as resolved.
Show resolved Hide resolved
}
}

.submit-row {
display: block;
text-align: right;
text-align: end;
input {
display: inline-block;
}
Expand All @@ -97,7 +102,7 @@ a.btn {
float: left;
}
a.deletelink {
margin-left: 10px;
margin-inline-start: 10px;
display: inline-block;
}
.btn {
Expand All @@ -108,10 +113,15 @@ a.btn {
vertical-align: top;
align-items: center;
& + .btn {
margin-left: 10px;
margin-inline-start: 10px;
}
}
}

[dir="rtl"] .submit-row .deletelink-box, .deletelink {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think .deletelink should be excluded from floating to the right:

Before:
image

After:
image

float: right;
}

.cms-btn-group {
@include button-variant($btn-default-color, $btn-default-bgcolor, $btn-default-border, true);
display: inline-block;
Expand All @@ -126,7 +136,7 @@ a.btn {
}
// last-child stands for advanced button
&:last-child {
margin-left: -4px;
margin-inline-start: -4px;
border-radius: 0 3px 3px 0 !important;
}
}
Expand Down Expand Up @@ -163,11 +173,15 @@ a.button.cancel-link {
display: inline-block;
line-height: 34px !important;
height: 34px !important;
margin-left: 10px;
margin-right: 10px;
margin-inline: 10px;
margin-top: 0;
padding: 0 20px !important
}

[dir="rtl"] a.button.cancel-link {
float: left;
}

// resets general list styles for object-tool list elements
.colM ul:not(.object-tools):not(.messagelist) {
margin: 20px 0 25px;
Expand All @@ -193,10 +207,9 @@ a.button.cancel-link {
}
// 3rd level list
ul {
margin-left: 20px;
margin-inline-start: 20px;
li {
padding-left: 0;
padding-right: 0;
padding-inline: 0;
}
}
}
Expand All @@ -209,7 +222,7 @@ p,
color: $gray !important;
font-weight: normal;
margin-bottom: 10px;
padding-left: 0;
padding-inline-start: 0;
}
.datetimeshortcuts {
font-size: $font-size-small !important;
Expand Down Expand Up @@ -324,7 +337,7 @@ ul.messagelist {
border-bottom: 0;
&:before {
position: absolute;
left: 10px;
inset-line-start: 10px;
fsbraun marked this conversation as resolved.
Show resolved Hide resolved
font-size: 18px !important;
margin-right: 20px !important;
}
Expand Down
68 changes: 40 additions & 28 deletions djangocms_admin_style/sass/components/_changelist.scss
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the text is not floating right:
image

I think it should start from the right direction here:
image

I'm not sure if this is the right file for this issue, but the navigator isn't quite right:
image
image

For reference here's how it looks in LTR:
image

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The padding on lines: 134, 156, and 255.
The margin on line: 155.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should changes to _changelist.scss be accompanied by changes to _tables.scss?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content attribute of changelist-filter details > summary::before should be changed to "←"
image

Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
.results #result_list {
box-shadow: none;
.related-widget-wrapper a.change-related {
margin-right: 0;
margin-left: 5px;
margin-inline-start: 5px;
margin-inline-end: 0;
}
}
#toolbartable .paginator {
Expand Down Expand Up @@ -76,9 +76,9 @@
overflow-y: auto;
width: $filtered-filter-width !important;
max-height: 585px;
margin-left: $filtered-filter-margin;
padding-right: $filtered-filter-padding;
padding-left: 0;
margin-inline-start: $filtered-filter-margin;
padding-inline-end: $filtered-filter-padding;
padding-inline-start: 0;

h2 {
letter-spacing: 0;
Expand All @@ -98,11 +98,11 @@
.paginator {
// calculates correct width because of margin right
width: calc(100% - #{$filter-changelist-margin}) !important;
border-right: 0;
border-inline-end: 0;
box-shadow: $base-box-shadow;
}
.actions {
margin-right: $filtered-filter-width !important;
margin-inline-end: $filtered-filter-width !important;
}

.changelist-form-container {
Expand Down Expand Up @@ -178,24 +178,30 @@
// makes sure that if admin cations are visible that toolbar floats to position next to actions #275, #285
&.actions-visible {
float: left;
[dir="rtl"] & {
float: right;
}
margin-bottom: 5px !important;
}
}
#changelist-filter {
// resets position to float filter next to changelist
position: static;
float: right;
[dir="rtl"] & {
float: left;
}
min-height: 100%;
margin-top: 8px;
border-left: 0;
border-top-right-radius: $border-radius-base;
border-bottom-right-radius: $border-radius-base;
border-start-end-radius: $border-radius-base;
border-end-end-radius: $border-radius-base;
background-color: $gray-lightest !important;
h2,
h3 {
margin-bottom: 15px !important;
padding-bottom: 10px !important;
padding-left: 0 !important;
padding-inline-start: 0 !important;
border-bottom: solid 1px $gray-lighter;
}
h2 {
Expand All @@ -215,8 +221,8 @@
ul {
font-size: $font-size-small;
margin-bottom: 10px;
margin-left: 0 !important;
padding-left: 0;
margin-inline-start: 0 !important;
padding-inline-start: 0;
padding-bottom: 0;
border-bottom: none;
}
Expand All @@ -225,9 +231,9 @@
margin-bottom: 15px;
&.selected {
font-weight: bold;
margin-left: 0 !important;
padding-left: 0 !important;
border-left: 0 !important;
margin-inline-start: 0 !important;
padding-inline-start: 0 !important;
border-inline-start: 0 !important;
a {
color: $color-primary !important;
}
Expand All @@ -245,7 +251,7 @@
width: 257px !important;
font-size: 12px;
margin-top: 3px;
margin-right: 10px;
margin-inline-end: 10px;
padding: 6px 40px 6px 20px;
border: 1px solid $gray-lighter;
border-radius: $border-radius-base;
Expand All @@ -255,15 +261,15 @@
// make sure that only icon is displayed
font: 0/0 a;
margin-bottom: 0;
margin-left: 0;
margin-inline-start: 0;
}
button {
font-family: $base-font-family;
font-size: $font-size-small;
vertical-align: bottom;
height: 36px;
margin-top: 0;
margin-right: 10px;
margin-inline-end: 10px;
margin-bottom: 0;
padding: 10px 15px !important;
box-shadow: none;
Expand All @@ -286,14 +292,17 @@
li {
display: inline;
float: left;
[dir="rtl"] & {
float: right;
}
color: $gray-darkest;
font-size: $font-size-normal;
font-weight: normal;
text-transform: uppercase;
margin-left: 25px;
margin-inline-start: 25px;
cursor: pointer;
&:first-child {
margin-left: 0;
margin-inline-start: 0;
}
}
a {
Expand Down Expand Up @@ -327,7 +336,7 @@
vertical-align: middle;
margin-top: 2px;
// adds space between search field and checkbox #216
margin-left: 10px;
margin-inline-start: 10px;
+ label {
display: inline-block !important;
color: $gray-light !important;
Expand All @@ -347,7 +356,7 @@
vertical-align: middle;
text-indent: -9999px;
box-sizing: border-box;
margin-left: 10px !important;
margin-inline-start: 10px !important;
padding: 10px 16px !important;
background-image: url("../img/[email protected]") !important;
background-repeat: no-repeat !important;
Expand All @@ -373,21 +382,21 @@
text-transform: uppercase;
width: 175px;
margin-bottom: 5px;
padding-left: 25px;
border-left: 0;
padding-inline-start: 25px;
border-inline-start: 0;
background: url("../img/icon_arrow_down.png") no-repeat left center;
background-size: 25px;
+ #changelist-filter {
position: absolute;
top: 30px;
margin-top: 1px;
padding-left: 15px !important;
padding-inline-start: 15px !important;
}
}
// select field when multiple sites are available #201
#site-selector {
margin-top: -18px;
margin-left: 0;
margin-inline-start: 0;
}
}

Expand Down Expand Up @@ -419,14 +428,17 @@
}
.text {
float: none;
padding-right: 0;
padding-inline-end: 0;
a {
padding: 0 !important;
}
}
.sortoptions {
display: inline-block;
float: right;
[dir="rtl"] & {
float: left;
}
padding-top: 0;
a.sortremove {
&:after {
Expand Down Expand Up @@ -455,7 +467,7 @@ a.lang-code {
font-size: $font-size-small !important;
font-weight: bold !important;
text-transform: uppercase;
margin-right: 10px !important;
margin-inline-end: 10px !important;
padding: 2px 8px !important;
border: solid 1px $gray-lighter;
border-radius: 10px !important;
Expand Down
Loading
Loading