django-cms / djangocms-admin-style

django CMS Admin Style is a Django Theme tailored to the needs of django CMS.
http://www.django-cms.org/
Other
411 stars 116 forks source link

feat: Add RTL support to admin modal #525

Closed sakhawy closed 3 months ago

sakhawy commented 3 months ago

Description

Added RTL support to cms-admin-modal

Related resources

Screenshots

Screenshot from 2024-03-18 11-10-57 Screenshot from 2024-03-18 11-11-03 Screenshot from 2024-03-21 13-33-02

Checklist

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (7b7e5e5) to head (87c43e9). Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #525 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 2 2 Lines 33 33 Branches 3 3 ========================================= Hits 33 33 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

fsbraun commented 3 months ago

@sakhawy Are the buttons and header positioned correctly?

sakhawy commented 3 months ago

Oh, I missed that! I'll adjust it, thanks!

sakhawy commented 3 months ago

@fsbraun Should I make this adjustment from djangocms-admin-style? Or would it better be a django-cms PR since the styling is in django-cms/cms/static/cms/sass/components/_modal.scss?

https://github.com/django-cms/django-cms/blob/21d6a6defc5a6321bd691e43eb57c112508b952f/cms/static/cms/sass/components/_modal.scss#L150-L170

.cms-modal-minimize,
.cms-modal-close,
.cms-modal-maximize {
    display: block;
    position: absolute;
    top: 50%;
    margin-top: math.div(-$modal-header-button-area-size, 2);
    right: $padding-normal;
    color: $gray-light;
    text-align: center;
    width: $modal-header-button-area-size;
    height: $modal-header-button-area-size;
    cursor: pointer;
    &:before {
        position: relative;
        top: math.div($modal-header-button-area-size - $icon-size, 2);
    }
    &:hover {
        color: $color-primary;
    }
}
fsbraun commented 3 months ago

You're right, of course.

sakhawy commented 3 months ago

@fsbraun inline-start and inline-end are not supported on relatively older browsers (as recent as 2023): https://caniuse.com/mdn-css_properties_float_inline-start so, I wanted to be extra careful. Should I make the change?

joshyu commented 3 months ago

@fsbraun , I think it's not a best practice to use too many !important in the CSS codes, which prevent us from customize those styes easily.

fsbraun commented 3 months ago

@joshyu I fully agree. djangocms-admin-style is not a good example for this, but has gone down that road may years ago. Alas, I believe with a bit of work this is fixable in most cases since all admin pages have the class djangocms-admin-style added to them allowing to override other styles by using higher specificity. It would be great if someone addressed this.

As a simpler work-around, I have created my own djangocms-simple-admin-style project (available on pypi): It tries to keep as many original django styles as possible and to void !important... Downside: It only works on browsers supporting CSS nesting (https://caniuse.com/css-nesting)