Moodle
  1. Moodle
  2. MDL-29367

Lines between RSS feed items are not showing in the Formal White, and other themes

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.1
    • Fix Version/s: 2.0.8, 2.1.5, 2.2.2
    • Component/s: Themes
    • Labels:
      None
    • Environment:
      2.1.1+, Build: 20110831, PHP Version 5.3.6, MySQL 5.1.57.
    • Rank:
      18890

      Description

      Lines are missing between feed items in the Formal White theme (see attached graphic). I noticed that other themes also have this problem. The "Standard" theme does include lines, which is why I am reporting this as a "bug".

      I suggest that the line be the same with as those between topics (see graphic). The line that is used to separate Upcoming Events might be too wide.

      I had already created a more general tracker item MDL-28561 (see attachment), which was "moved" to a theme problem. Then, in MDL-29211, it was suggested that I start a new tracker item so that this line issue could be added to the similar line issue about forum posts.

        Issue Links

          Activity

          Hide
          Daniele Cordella added a comment -

          Message for HQ developers or HQ integrators.
          The current "upcoming events" separator is <hr />.
          AFAIK this is not always used to separate lists.
          I have two comments about this:
          1. the reporter of this issue complains about its style (See description of this issue: "The line that is used to separate Upcoming Events might be too wide.")
          2. I feel a more homogeneous moodle is more appealing for each user.
          Btw: in this patch I dropped the display of the <hr />. If you drop the <hr /> from the code, please, correct my style at line 249 of formal_white.css too.

          Show
          Daniele Cordella added a comment - Message for HQ developers or HQ integrators. The current "upcoming events" separator is <hr />. AFAIK this is not always used to separate lists. I have two comments about this: 1. the reporter of this issue complains about its style (See description of this issue: "The line that is used to separate Upcoming Events might be too wide.") 2. I feel a more homogeneous moodle is more appealing for each user. Btw: in this patch I dropped the display of the <hr />. If you drop the <hr /> from the code, please, correct my style at line 249 of formal_white.css too.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Sam Hemelryk added a comment -

          Thanks Daniele this has been integrated now.

          I've had to fix up your commit messages on all issues today, could you please in the future remember to structure those commit messages as per the coding guidelines.

          For the formal white issues you've fixed this week message should be something like:

          MDL-29367 theme_formal_white: Lines between RSS and upcoming events

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Daniele this has been integrated now. I've had to fix up your commit messages on all issues today, could you please in the future remember to structure those commit messages as per the coding guidelines. For the formal white issues you've fixed this week message should be something like: MDL-29367 theme_formal_white: Lines between RSS and upcoming events Cheers Sam
          Hide
          Jason Fowler added a comment -

          Testing instructions and description of the fix should have been more clear ... I tried testing this under some "other themes" ... and they all still exhibit the issue...

          Show
          Jason Fowler added a comment - Testing instructions and description of the fix should have been more clear ... I tried testing this under some "other themes" ... and they all still exhibit the issue...
          Hide
          Jason Fowler added a comment -

          Passed tests in all versions.

          After looking at the committed code I was able to figure out that only the formal white theme has been fixed. Have issues for all the other themes been raised too?

          Show
          Jason Fowler added a comment - Passed tests in all versions. After looking at the committed code I was able to figure out that only the formal white theme has been fixed. Have issues for all the other themes been raised too?
          Hide
          Daniele Cordella added a comment - - edited

          Ciao Jason.
          In Italy we love to say "Stai sfondando una porta aperta" that may be translated as: "You are breaking through an open door".
          As I wrote before, this patch is really a patch and not a general solution because moodle core starts by writing a <hr /> and later continues hiding it.
          To fix this issue in the correct way, IMHO, it should be:
          -> dropped off the ugly <hr /> from moodle core
          -> added my FW patch at global/general level (in canvas or in base theme).
          As usual: this is my 1 cent contribution. The floor at moodle HQ.
          (sorry for my bloody english)

          Show
          Daniele Cordella added a comment - - edited Ciao Jason. In Italy we love to say "Stai sfondando una porta aperta" that may be translated as: "You are breaking through an open door". As I wrote before, this patch is really a patch and not a general solution because moodle core starts by writing a <hr /> and later continues hiding it. To fix this issue in the correct way, IMHO, it should be: -> dropped off the ugly <hr /> from moodle core -> added my FW patch at global/general level (in canvas or in base theme). As usual: this is my 1 cent contribution. The floor at moodle HQ. (sorry for my bloody english)
          Hide
          Daniele Cordella added a comment -

          @Sam: Sure Sam. Sorry. I should really go to read coding guidelines in better detail. I apologize.

          Show
          Daniele Cordella added a comment - @Sam: Sure Sam. Sorry. I should really go to read coding guidelines in better detail. I apologize.
          Hide
          Jason Fowler added a comment -

          Daniele, Don't apologize for poor English, your English is infinitely better than my Spanish, and I appreciate your efforts to communicate in English

          Show
          Jason Fowler added a comment - Daniele, Don't apologize for poor English, your English is infinitely better than my Spanish, and I appreciate your efforts to communicate in English
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!).

          icao_reverse('arreis olik rebemevon afla letoh ognat');
          

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!). icao_reverse('arreis olik rebemevon afla letoh ognat'); Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: