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

use aside for margin-note #235

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

spdegabrielle
Copy link
Sponsor Member

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

address racket#205 
Note `aside` cannot be used for the inline version `margin-note*`
@spdegabrielle
Copy link
Sponsor Member Author

@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
Copy link
Sponsor Member Author

Hello! Any feedback?

@spdegabrielle
Copy link
Sponsor Member Author

ping, any thoughts on this? @shriram

@sorawee
Copy link
Contributor

sorawee commented Jun 11, 2020

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
Copy link
Sponsor Member Author

@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
Copy link
Contributor

sorawee commented Jun 11, 2020

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
Copy link
Sponsor Member Author

Ok.

@spdegabrielle
Copy link
Sponsor Member Author

@sorawee done.

@sorawee
Copy link
Contributor

sorawee commented Jun 15, 2020

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
Copy link
Sponsor Member Author

Thank you @sorawee your diligence is appreciated!

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

@spdegabrielle
Copy link
Sponsor Member Author

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?)

@sorawee sorawee added this to the 8.12 milestone Nov 9, 2023
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

2 participants