racket / scribble

Other
201 stars 92 forks source link

use `aside` for `margin-note` #235

Open spdegabrielle opened 4 years ago

spdegabrielle commented 4 years ago

address #205 Note aside cannot be used for the inline version margin-note*

spdegabrielle commented 4 years ago

@shriram from your suggestion this pr makes scribble use the aside tag for margin notes

this (from more.scrbl)

@margin-note{Set your @tt{PATH} environment variable so you can use
 @exec{raco} and other Racket command line functions. On Mac OS:
 @tt{sudo sh -c @literal{'}echo "/Applications/Racket v@version{}/bin" 
 >> /etc/paths.d/racket@literal{'}} (This assumes you have installed
 Racket in the @tt{Applications} folder). On Windows: add the racket
 @tt{bin} path to @onscreen{Path} in @onscreen{Environment Variables}
 (under @onscreen{System Properties}, @onscreen{Advanced} tab)}

renders as the following html with no visible difference.

            <blockquote class="refpara">
                <blockquote class="refcolumn">
                    <aside class="refcontent">
                        <p>
                            Set your 
                            <span class="stt">PATH</span>
                             environment variable so you can use
                            <span class="stt">raco</span>
                             and other Racket command line functions. On Mac OS:
                            <span class="stt">sudo sh -c </span>
                            '
                            <span class="stt">echo "/Applications/Racket v</span>
                            <span class="stt">7.7</span>
                            <span class="stt">/bin"</span>
                            <span class="stt"></span>
                            <span class="stt">&gt;&gt; /etc/paths.d/racket</span>
                            ' (This assumes you have installed
                            Racket in the 
                            <span class="stt">Applications</span>
                             folder). On Windows: add the racket
                            <span class="stt">bin</span>
                             path to 
                            <span class="ssansserif">Path</span>
                             in 
                            <span class="ssansserif">Environment Variables</span>

                            (under 
                            <span class="ssansserif">System Properties</span>
                            , 
                            <span class="ssansserif">Advanced</span>
                             tab)
                        </p>
                    </aside>
                </blockquote>
            </blockquote>
spdegabrielle commented 4 years ago

Hello! Any feedback?

spdegabrielle commented 4 years ago

ping, any thoughts on this? @shriram

sorawee commented 4 years ago

I think having aside as the outermost tag makes more sense. It means that the whole block is an aside content, rather than "here's a blockquote, and what lies in the blockquote is an aside content".

  (make-nested-flow
   (make-style (if left? "refparaleft" "refpara")
               (cons (alt-tag "aside") '(command never-indents)))
   (list
    (make-nested-flow
     (make-style (if left? "refcolumnleft" "refcolumn")
                 null)
     (list
      (make-nested-flow
       (make-style "refcontent" null)
       (decode-flow c))))))
spdegabrielle commented 4 years ago

@sorawee Sorry I did the minimum to provide the fix.

I didn't have time to unpick the repeated use of make-nested-flow that renders as three levels of blockquote. The one I chose to change to aside didn't see to rely on the default browser styling of blockquote, and rendered the same. (It mays still go wrong on other devices but I'm assuming safari is close enough to chrome, IE. Edge in a desktop context. (I'm hoping mobile use is rare and/or passable)

I know the multiple nested-content seems excessive but I have to assume the author of the current renderer and CSS styles had a good reason.

kr Stephen

PS In my heart I want to do this at pure semantic html and css styles but It is major project and I'm no expert on css/html - so I don't even know if that is a realistic goal:

I can't turn

   <blockquote class="refpara">
                <blockquote class="refcolumn">
                    <aside class="refcontent">

into

<aside class="refpara refcolumn refcontent">

because at the very least because margins are summative.

(and I had to look up if I needed spaces or commas in the class attribute - I still don't know if the order is right.)

sorawee commented 4 years ago

I was suggesting to turn your proposed change:

   <blockquote class="refpara">
                <blockquote class="refcolumn">
                    <aside class="refcontent">

to:

   <aside class="refpara">
                <blockquote class="refcolumn">
                    <blockquote class="refcontent">
spdegabrielle commented 4 years ago

Ok.

spdegabrielle commented 4 years ago

@sorawee done.

sorawee commented 4 years ago

Apparently aside doesn't work correctly on IE8 and older versions. There's a hack to make it work, however. Put:

<script>
  document.createElement('aside');
</script>

inside <head>...</head>.

See also https://weblog.west-wind.com/posts/2015/jan/12/main-html5-tag-not-working-in-internet-explorer-91011#Internet-Explorer-8-and-older

spdegabrielle commented 4 years ago

Thank you @sorawee your diligence is appreciated!

I think this is the correct fix but I'm having trouble testing.

spdegabrielle commented 4 years ago

adding (script ([type "text/javascript"]) "\ndocument.createElement('aside');\n") to line 880 of scribble-lib/scribble/html-render.rkt didn't seem to have any impact on the rendered html. I must have changed the wrong one - but I've been unable to determine where the right one is. (and what is html-render.rkt for? if not for this?)