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

[4.0] fix error of styling issue with save and close button #27816

Merged
merged 6 commits into from Feb 6, 2020
Merged

[4.0] fix error of styling issue with save and close button #27816

merged 6 commits into from Feb 6, 2020

Conversation

Subhang23
Copy link
Contributor

@Subhang23 Subhang23 commented Feb 5, 2020

Pull Request for Issue #26771

Summary of Changes

Fixed styling issue with Save & Close button on mobile on article page

Testing Instructions

Expected result

The content in "Save and Close" button should be aligned properly.

Actual result

Screenshot from 2020-02-05 18-24-09

Documentation Changes Required

@richard67
Copy link
Member

@Subhang23 Our Pull Request (PR) template provides 2 pre-defined section, "Expected result" and "Actual result". "Expected result" shall describe what is the right, expected behavior which is approached with the PR, i.e. the result after having applied the PR. "Actual result" shall describe the current, actual behavior of Joomla (3 or 4, depending on which version the PR is made for, in case of this PR: 4).

Now you used "Present result" and "Actual result" in this PR.

Due to a certain ambiguity of the English language for that word, "Present" can be a verb and mean "to show something to someone", but it also can be an adjective and mean (almost) same as "current" or "actual". So for a quick reader it is not really clear which of your screenshots shows the situation before and which one shows the sitation after applying your PR.

Of course one could test to find it out, but for readers it would be better if it would be more clear.

Could you correct the section headings above your screenshots so that it is like I described above, "Actual result" for what we have in Joomla 4 now, and "Expected result" for how it with your PR aplied?

Thanks in advance.

@brianteeman
Copy link
Contributor

The intended design is for all those buttons to be 100% width on a mobile

@Subhang23
Copy link
Contributor Author

@richard67 I have changed it and am sorry for the improper message.

@Subhang23
Copy link
Contributor Author

@brianteeman Would this be good?
Screenshot from 2020-02-05 22-07-43

@brianteeman
Copy link
Contributor

yes that is the intended design

@infograf768
Copy link
Member

Looks good but has to be tested in RTL and with other pages.

@brianteeman
Copy link
Contributor

Unfortunately this breaks the action button

image

@brianteeman
Copy link
Contributor

I should note that I dont understand why these two dropdown buttons are coded differently #27767

@Subhang23
Copy link
Contributor Author

@brianteeman @richard67 @infograf768 Is this fine?

@Quy
Copy link
Contributor

Quy commented Feb 5, 2020

Issue with RTL language. Install Persian to test.

27816

@richard67
Copy link
Member

@richard67 I have changed it and am sorry for the improper message.

No need to say sorry. I've explained how we normally do it, you followed that, so all is fine.

@Subhang23
Copy link
Contributor Author

@Quy Is this good?
Screenshot from 2020-02-06 01-33-39

@infograf768
Copy link
Member

Almost there.
Remains to modify line 383 of /administrator/templates/atum/scss/blocks/_toolbar.scss
from

     @include media-breakpoint-down(sm) {
        [dir=rtl] & {
          margin-left: 0.75rem;
          margin-right: 0 !important;
        }
      }

to

     @include media-breakpoint-down(sm) {
        [dir=rtl] & {
          margin-left: 0.75rem !important;         // Added !important
          margin-right: 0 !important;
        }
      }

before this modification, we get

marginleft_saveandclose

after this modification, we get

marginleft_saveandclose_after

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 50c28d3

Looks OK now to me.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27816.

@Quy
Copy link
Contributor

Quy commented Feb 6, 2020

Remove right margin.

27816

@infograf768
Copy link
Member

@Quy
Not sure we can change that margin, comparing with other views.
In any case, I think it is unrelated to the original reason of this PR.
I suggest to go with what we have here and then see what we can do, if possible, in another PR.

@Quy
Copy link
Contributor

Quy commented Feb 6, 2020

I have tested this item ✅ successfully on 50c28d3


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27816.

@Quy
Copy link
Contributor

Quy commented Feb 6, 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27816.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 6, 2020
@infograf768 infograf768 merged commit 9a53a31 into joomla:4.0-dev Feb 6, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 6, 2020
@infograf768
Copy link
Member

tks

@infograf768 infograf768 added this to the Joomla 4.0 milestone Feb 6, 2020
@infograf768 infograf768 changed the title fix error of styling issue with save and close button [4.0] fix error of styling issue with save and close button Feb 6, 2020
@infograf768
Copy link
Member

Please test #27841 solving all alignments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants