Uploaded image for project: 'Forrest'
  1. Forrest
  2. FOR-604

The document-v* warning element does not get rendered properly

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.8
    • Fix Version/s: 0.8
    • Component/s: None
    • Labels:
      None

      Description

      The "warning" element is rendered differently to the "note" element. Perhaps it only happens with the "pelt" skin - not yet verified. See screenshot.
      1. 604-screenshot.png
        29 kB
        David Crossley
      2. basic.css.diff
        0.7 kB
        Gavin
      3. basic.css.diff
        0.9 kB
        Gavin

        Activity

        Hide
        gmcdonald Gavin added a comment -
        This has been put in by design it seems, the leather-dev basic.css has it also as well as the pelt basic.css.
        The 'Note' element used for both 'Note' and 'Important' has extra padding, line-height and font-size attributes.

        This is the pelt/css/basic.css

        .note, .warning, .fixme {
          border: solid black 1px;
          margin: 1em 3em;
        }

        .note .label {
          background: #369;
          color: white;
          font-weight: bold;
          padding: 5px 10px;
        }
        .note .content {
          padding: 5px 10px;
          background: #F0F0FF;
          color: black;
          line-height: 120%;
          font-size: 90%;
        }
        .warning .label {
          background: #C00;
          color: white;
        }
        .warning .content {
          background: #FFF0F0;
          color: black;
        }
        .fixme .label {
          background: #C6C600;
        }

        The leather-dev/css/basic.css is the same with one exception :

        .note .content is set at 170% whereas in pelt it is 120%.

        So either you leave it by design, or add padding, line-height and font-size to the .warning and .fixme, or remove those from .note.

        Let me know and I'll make the changes if you like.

        Check out http://localhost:8888/samples/sample.html also.

        Gav...
        Show
        gmcdonald Gavin added a comment - This has been put in by design it seems, the leather-dev basic.css has it also as well as the pelt basic.css. The 'Note' element used for both 'Note' and 'Important' has extra padding, line-height and font-size attributes. This is the pelt/css/basic.css .note, .warning, .fixme {   border: solid black 1px;   margin: 1em 3em; } .note .label {   background: #369;   color: white;   font-weight: bold;   padding: 5px 10px; } .note .content {   padding: 5px 10px;   background: #F0F0FF;   color: black;   line-height: 120%;   font-size: 90%; } .warning .label {   background: #C00;   color: white; } .warning .content {   background: #FFF0F0;   color: black; } .fixme .label {   background: #C6C600; } The leather-dev/css/basic.css is the same with one exception : .note .content is set at 170% whereas in pelt it is 120%. So either you leave it by design, or add padding, line-height and font-size to the .warning and .fixme, or remove those from .note. Let me know and I'll make the changes if you like. Check out http://localhost:8888/samples/sample.html also. Gav...
        Hide
        crossley David Crossley added a comment -
        Thanks for the research. I reckon it is not by deliberate design, rather a matter of inconsistent evolution. Glancing at the history
        http://svn.apache.org/viewcvs.cgi/forrest/trunk/main/webapp/skins/pelt/css/basic.css
        it seems that we had a warning and fixme defined in a certain way, and "frame" used for notes in another way.

        Yes please Gav, those box types should all be consistent, so please tweak the CSS to make them all look good like Note does.
        Show
        crossley David Crossley added a comment - Thanks for the research. I reckon it is not by deliberate design, rather a matter of inconsistent evolution. Glancing at the history http://svn.apache.org/viewcvs.cgi/forrest/trunk/main/webapp/skins/pelt/css/basic.css it seems that we had a warning and fixme defined in a certain way, and "frame" used for notes in another way. Yes please Gav, those box types should all be consistent, so please tweak the CSS to make them all look good like Note does.
        Hide
        gmcdonald Gavin added a comment -
        Ok, I have made the changes needed. You can view it in action, I have uploaded a basic site and at http://apache.minitutorials.com/samples/sample.html you can see the new CSS changes in action.

        I am not all that familiar with svn, although it did state that basic.css could not be diff'd (or something like that)

        If somebody wants to explain how I can commit these changes, if I can not then please paste the following into /pelt/css/basic.css as a replacement for the code in my earlier message above.

        .note, .warning, .fixme {
          border: solid black 1px;
          margin: 1em 3em;
        }

        .note .label {
          background: #369;
          color: white;
          font-weight: bold;
            padding: 5px 10px;
        }
        .note .content {
          background: #F0F0FF;
          color: black;
          line-height: 120%;
          font-size: 90%;
            padding: 5px 10px;
        }
        .warning .label {
          background: #C00;
          color: white;
          font-weight: bold;
            padding: 5px 10px;
        }
        .warning .content {
          background: #FFF0F0;
          color: black;
          line-height: 120%;
          font-size: 90%;
            padding: 5px 10px;
        }
        .fixme .label {
          background: #C6C600;
          font-weight: bold;
            padding: 5px 10px;
        }
        .fixme .content {
        padding: 5px 10px;
        }

        -----------------------------------------------------------------------------------------------
        I guess to be consistent then leather-dev/css/basic.css will also need to
        be changed the same way. I will load leather-dev and make the changes,
        making sure that it does not break anything else and then post back here.

        Is there anything else I need to do here, doc changes ?

        Gav...
        Show
        gmcdonald Gavin added a comment - Ok, I have made the changes needed. You can view it in action, I have uploaded a basic site and at http://apache.minitutorials.com/samples/sample.html you can see the new CSS changes in action. I am not all that familiar with svn, although it did state that basic.css could not be diff'd (or something like that) If somebody wants to explain how I can commit these changes, if I can not then please paste the following into /pelt/css/basic.css as a replacement for the code in my earlier message above. .note, .warning, .fixme {   border: solid black 1px;   margin: 1em 3em; } .note .label {   background: #369;   color: white;   font-weight: bold;     padding: 5px 10px; } .note .content {   background: #F0F0FF;   color: black;   line-height: 120%;   font-size: 90%;     padding: 5px 10px; } .warning .label {   background: #C00;   color: white;   font-weight: bold;     padding: 5px 10px; } .warning .content {   background: #FFF0F0;   color: black;   line-height: 120%;   font-size: 90%;     padding: 5px 10px; } .fixme .label {   background: #C6C600;   font-weight: bold;     padding: 5px 10px; } .fixme .content { padding: 5px 10px; } ----------------------------------------------------------------------------------------------- I guess to be consistent then leather-dev/css/basic.css will also need to be changed the same way. I will load leather-dev and make the changes, making sure that it does not break anything else and then post back here. Is there anything else I need to do here, doc changes ? Gav...
        Hide
        gmcdonald Gavin added a comment -
        I have attatched basic.css.diff for inclusion.

        Thanks

        Gav...
        Show
        gmcdonald Gavin added a comment - I have attatched basic.css.diff for inclusion. Thanks Gav...
        Hide
        crossley David Crossley added a comment -
        Thanks, applied your patch. I notice that there was unnecessary whitespace added, so i removed that. This will probably mean that you (Gavin) will get conflicts when you do 'svn update'. So either do 'svn revert basic.css' beforehand, or move the file out of the way and do 'svn update' again.

        I will leave this issue open in case you manage to do leather-dev css too.

        For the record, Gavin' s other questions above were answered on the dev list.
        Show
        crossley David Crossley added a comment - Thanks, applied your patch. I notice that there was unnecessary whitespace added, so i removed that. This will probably mean that you (Gavin) will get conflicts when you do 'svn update'. So either do 'svn revert basic.css' beforehand, or move the file out of the way and do 'svn update' again. I will leave this issue open in case you manage to do leather-dev css too. For the record, Gavin' s other questions above were answered on the dev list.
        Hide
        gmcdonald Gavin added a comment -
        This 'basic.css' not to be confused with the first pelt one, contains my amendments to the original /leather-dev/css/basic.css to match those of the pelt skin. I am not sure if this skin is being kept or not, but is done anyway. The new leather-dev skin codenamed viewHelper , for use with contracts has a different css style sheet which seems to inherit from viewHelper plugin default.css. Taking a look at http://apache.minitutorials.com/samples/sample.html shows similar problems in that 'fixme' does not have a style. Also the table in the preceding example does not show. Should I go ahead and fix the 'fixme' to match, if so do I use the build version or the whiteboard version.

        Gav...
        Show
        gmcdonald Gavin added a comment - This 'basic.css' not to be confused with the first pelt one, contains my amendments to the original /leather-dev/css/basic.css to match those of the pelt skin. I am not sure if this skin is being kept or not, but is done anyway. The new leather-dev skin codenamed viewHelper , for use with contracts has a different css style sheet which seems to inherit from viewHelper plugin default.css. Taking a look at http://apache.minitutorials.com/samples/sample.html shows similar problems in that 'fixme' does not have a style. Also the table in the preceding example does not show. Should I go ahead and fix the 'fixme' to match, if so do I use the build version or the whiteboard version. Gav...
        Hide
        crossley David Crossley added a comment -
        Thanks, applied. I am closing this now.

        Regarding your other questions about viewHelper plugin, i suggest that your CSS skills would be best utilised to help Cyriaque, Thorsten, Diwaker, and others to generally improve the use of CSS within "views". I am not clear about its use of the leather-dev "skin", so best talk to them on the dev list.

        By the way, this patch would have been better created from the top-level, like you did with the pelt one (cd forrest; svn diff main/webapp/skins/pelt > pelt-css.diff). In that way, the pathname of the files is in the diff and so i cannot possibly apply to the wrong source. This one just contained the name "basic.css" so is obscure.

        I see the problem with the whitespace is that the rest of the file is inconsistent.
        Show
        crossley David Crossley added a comment - Thanks, applied. I am closing this now. Regarding your other questions about viewHelper plugin, i suggest that your CSS skills would be best utilised to help Cyriaque, Thorsten, Diwaker, and others to generally improve the use of CSS within "views". I am not clear about its use of the leather-dev "skin", so best talk to them on the dev list. By the way, this patch would have been better created from the top-level, like you did with the pelt one (cd forrest; svn diff main/webapp/skins/pelt > pelt-css.diff). In that way, the pathname of the files is in the diff and so i cannot possibly apply to the wrong source. This one just contained the name "basic.css" so is obscure. I see the problem with the whitespace is that the rest of the file is inconsistent.

          People

          • Assignee:
            crossley David Crossley
            Reporter:
            crossley David Crossley
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development