Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-39683

RTL Clean/Standard alignment issues in the message screen

    Details

    • Testing Instructions:
      Hide
      • Test in Clean and Standard
      1. Go into your user's Messaging page.
      2. Send a few messages to any user.
      3. Login as that other user and send some messages to yourself (you can reply your previous message).
      4. Go back being you.
      5. Switch language to Hebrew (or any other RTL language).
      6. Make sure that your personal picture is displayed on the right side of the screen and the other user's picture on the left.
      7. Make sure that the messages are aligned the name of the user who sent them.
      Show
      Test in Clean and Standard Go into your user's Messaging page. Send a few messages to any user. Login as that other user and send some messages to yourself (you can reply your previous message). Go back being you. Switch language to Hebrew (or any other RTL language). Make sure that your personal picture is displayed on the right side of the screen and the other user's picture on the left. Make sure that the messages are aligned the name of the user who sent them.
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-39683-master

      Description

      There are some alignment issues on /message/index.php. Not sure if this is due to using an RTL language, the Clean theme or the combination of both. Attaching screenshots of English/Standard Vs Arabic/Clean.

        Gliffy Diagrams

          Attachments

          1. after.png
            after.png
            244 kB
          2. before.png
            before.png
            244 kB
          3. LTRStandardMessaging.png
            LTRStandardMessaging.png
            170 kB
          4. message.png
            message.png
            34 kB
          5. RTLCleanMessaging.png
            RTLCleanMessaging.png
            157 kB

            Activity

            Hide
            andyjdavis Andrew Davis added a comment -

            The phrase "Message" next to the text box has been pushed onto a separate line.

            The text box isnt centred.

            The send message button isnt centred.

            Messages are meant to be listed under the user that sent them. That appears to be a bit out of whack.

            Show
            andyjdavis Andrew Davis added a comment - The phrase "Message" next to the text box has been pushed onto a separate line. The text box isnt centred. The send message button isnt centred. Messages are meant to be listed under the user that sent them. That appears to be a bit out of whack.
            Hide
            andyjdavis Andrew Davis added a comment -

            This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.

            For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

            Show
            andyjdavis Andrew Davis added a comment - This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment
            Hide
            poltawski Dan Poltawski added a comment -

            Hi,

            Sorry, i'm afraid i've played with this and I don't see how this change is fixing anything? I can't spot it.

            Also reading the testing instructions, it looks to me like its working as per the testing instructions beforehand?

            I've attached screenshots of before/after the patch - and I just can't see the change.

            Show
            poltawski Dan Poltawski added a comment - Hi, Sorry, i'm afraid i've played with this and I don't see how this change is fixing anything? I can't spot it. Also reading the testing instructions, it looks to me like its working as per the testing instructions beforehand? I've attached screenshots of before/after the patch - and I just can't see the change.
            Hide
            cibot CiBoT added a comment -

            Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

            Show
            cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            fred Frédéric Massart added a comment -

            I'm sending this for peer review, as Dan I had troubles understanding what Nadav's patch was doing. In fact, without the patch the messages are aligned with the wrong user... the messages from User A appear underneath the ones of User B. I picked the patch in my branches and pushed it. Thanks Nadav.

            Show
            fred Frédéric Massart added a comment - I'm sending this for peer review, as Dan I had troubles understanding what Nadav's patch was doing. In fact, without the patch the messages are aligned with the wrong user... the messages from User A appear underneath the ones of User B. I picked the patch in my branches and pushed it. Thanks Nadav.
            Hide
            fred Frédéric Massart added a comment -

            I added a commit because I noticed that this was already an issue on Standard.

            Show
            fred Frédéric Massart added a comment - I added a commit because I noticed that this was already an issue on Standard.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Nadav and Fred,

            Thank you for working on this. As Fred mentioned above, after applying the patch, I could only distinguished the different on the message alignment.

            Some additional note, the UI for this page could be improved by clarifying which message is written by which user (currently, it indicates by timestamps).

            Other than that, the patch looks great.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Nadav and Fred, Thank you for working on this. As Fred mentioned above, after applying the patch, I could only distinguished the different on the message alignment. Some additional note, the UI for this page could be improved by clarifying which message is written by which user (currently, it indicates by timestamps). Other than that, the patch looks great.
            Hide
            fred Frédéric Massart added a comment -

            Hi Rosie,

            I am not sure to understand what you mean with the timestamps. Could you illustrate with a screenshot please?

            Many thanks!
            Fred

            Show
            fred Frédéric Massart added a comment - Hi Rosie, I am not sure to understand what you mean with the timestamps. Could you illustrate with a screenshot please? Many thanks! Fred
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Fred,

            I attached a screenshot for the message page.

            Instead of using the user's name as indication, it used time (eg: 3:24 am) as the label.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Fred, I attached a screenshot for the message page. Instead of using the user's name as indication, it used time (eg: 3:24 am) as the label.
            Hide
            fred Frédéric Massart added a comment -

            Oh I understand what you mean, thanks for clarifying. Surely this interface is a bit outdated now, but I'm afraid that this is out of the scope of this issue. You are right though, this could be highly improved, and benefit from proper responsive design. Pushing this for integration.

            Show
            fred Frédéric Massart added a comment - Oh I understand what you mean, thanks for clarifying. Surely this interface is a bit outdated now, but I'm afraid that this is out of the scope of this issue. You are right though, this could be highly improved, and benefit from proper responsive design. Pushing this for integration.
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Nadav and Fred. Integrated to 25, 26 and master. Appears to work correctly for me.

            Show
            damyon Damyon Wiese added a comment - Thanks Nadav and Fred. Integrated to 25, 26 and master. Appears to work correctly for me.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Fred - tested and passed

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Fred - tested and passed
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I won't be saying "Thanks!" this week. I'm tired of it.

            For the good (and the bad), your code is now part of Moodle, the best LMS in the world. Hope you are contributing for that to continue being a fact (and not the opposite), sincerely.

            Just closing this as fixed, ciao

            PS: Just a bit of black/cruel humor, sorry, LOL!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I won't be saying "Thanks!" this week. I'm tired of it. For the good (and the bad), your code is now part of Moodle, the best LMS in the world. Hope you are contributing for that to continue being a fact (and not the opposite), sincerely. Just closing this as fixed, ciao PS: Just a bit of black/cruel humor, sorry, LOL!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Mar/14