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

v5.1.0(pre2): Extra top space if parbox=false tcolorbox starts with a list #262

Closed
muzimuzhi opened this issue Jan 8, 2024 · 8 comments
Closed
Assignees
Labels

Comments

@muzimuzhi
Copy link
Contributor

Unfortunately, the fix for #171 caused regression of #123. The extra top space actually consists of vertical spacing of an empty paragraph (\baselineskip) plus \parskip.

Since the culprit is the insertion position of \noindent, different from that in #171 which was the color whatsit, I choose to open a new issue.

\documentclass{article}
\usepackage{tcolorbox}

\tcbset{set parbox/.style={parbox=#1, title={\texttt{parbox=#1}}}}

\begin{document}

% issue #123, extra top space in "parbox=false" tcolorbox starting with a list
\begin{tcolorbox}
  \begin{enumerate}
    \item abc
  \end{enumerate}
\end{tcolorbox}

\begin{tcolorbox}[set parbox=false]
  \begin{enumerate}
    \item abc
  \end{enumerate}
\end{tcolorbox}

% issue #171, no "before skip" if nested in a "parbox=false" tcolorbox
\begin{tcolorbox}
  content
  \begin{tcolorbox}[before skip=20pt]
    test \texttt{before skip balanced}
  \end{tcolorbox}
\end{tcolorbox}

\begin{tcolorbox}[set parbox=false]
  content
  \begin{tcolorbox}[before skip=20pt]
    test \texttt{before skip balanced}
  \end{tcolorbox}
\end{tcolorbox}

\end{document}

image

@T-F-S
Copy link
Owner

T-F-S commented Jan 8, 2024

Currently, I see no good solution here.

For an automatic solution I see no way. A manual way to remove the extra space, if the content starts with a list, is also not pleasant to implement, see special remove 1 and special remove 2:

\documentclass{article}
\usepackage{tcolorbox}

\tcbset{set parbox/.style={parbox=#1, title={\texttt{parbox=#1}}}}

\makeatletter
\tcbset{
  special remove 1/.style={%
      before upper={\vspace{-13.055555pt}},%     value ?
    },
  special remove 2/.code={%
      \appto\tcb@lateoptions@hook{\preto\kvtcb@before@upper{\let\tcb@set@parbox@indent\@empty}}%     upper
    },
}
\makeatother

\begin{document}

% issue #123, extra top space in "parbox=false" tcolorbox starting with a list
\begin{tcolorbox}
  \begin{enumerate}
    \item abc
  \end{enumerate}
\end{tcolorbox}

\begin{tcolorbox}[set parbox=false,
  % special remove 1,
  special remove 2,
]
  \begin{enumerate}
    \item abc
  \end{enumerate}
\end{tcolorbox}

% issue #171, no "before skip" if nested in a "parbox=false" tcolorbox
\begin{tcolorbox}
  content
  \begin{tcolorbox}[before skip=20pt]
    test \texttt{before skip balanced}
  \end{tcolorbox}
\end{tcolorbox}

\begin{tcolorbox}[set parbox=false]
  content
  \begin{tcolorbox}[before skip=20pt]
    test \texttt{before skip balanced}
  \end{tcolorbox}
\end{tcolorbox}

\end{document}


\documentclass{article}
\usepackage[skins]{tcolorbox}

\tcbset{set parbox/.style={parbox=#1, title={\texttt{parbox=#1}}}}

\begin{document}

\tcbset{size=minimal,draft}

% issue #123, extra top space in "parbox=false" tcolorbox starting with a list
\begin{tcolorbox}[set parbox=true]
  \begin{enumerate}
    \item abc
  \end{enumerate}
\end{tcolorbox}

\bigskip

\begin{tcolorbox}[set parbox=false]
  \begin{enumerate}
    \item abc
  \end{enumerate}
\end{tcolorbox}

\bigskip

\makeatletter
\begin{tcolorbox}[set parbox=false,
  %before upper={\vspace{-13.055555pt}},
  %code={\let\tcb@set@parbox@indent\@empty},
  %code={\appto\tcb@lateoptions@hook{\preto\kvtcb@before@upper{\let\tcb@set@parbox@indent\@empty}}},
  ]
  %\vspace{-13.055555pt}%
  %\show\tcb@lateoptions@hook
  %\show\kvtcb@before@upper
  \begin{enumerate}
    \item abc
  \end{enumerate}
\end{tcolorbox}
\makeatother

\bigskip


% issue #171, no "before skip" if nested in a "parbox=false" tcolorbox
\begin{tcolorbox}[set parbox=true]
  content
  \begin{tcolorbox}[before skip=20pt]
    test \texttt{before skip balanced}
  \end{tcolorbox}
\end{tcolorbox}

\bigskip


\begin{tcolorbox}[set parbox=false]
  content
  \begin{tcolorbox}[before skip=20pt]
    test \texttt{before skip balanced}
  \end{tcolorbox}
\end{tcolorbox}

\end{document}

The parbox=false option is documented as experimental setting. Maybe, I should make the warning more prominent...

@muzimuzhi
Copy link
Contributor Author

As a replacement of \noindent, \AddToHookNext{para/begin}{\OmitIndent}, initially proposed in your comment #171 (comment), works well when content is non-empty and not starts with another tcolorbox.

Here is a loose attempt which tries to not apply \OmitIndent in that two corner cases.

  • Any more corner cases?
  • Code added to hook by \AddToHookNext cannot specify a hook label and is executed after all codes added by \AddToHook, so the \OmitIndent from tcolorbox may not be executed early enough. Will this be a real problem?
Example v2, using \AddToHookNext{para/begin}{<if ... then \OmitIndent fi>}

\documentclass{article}
\usepackage{multicol}
\usepackage{tcolorbox}

\makeatletter
\ExplSyntaxOn
\def\tcb@set@parbox@indent@{
  % "\hook_gput_next_code:ne" is the same as "\ExpandArgs{ne} \AddToHookNext"
  \hook_gput_next_code:ne {para/begin}
    { \tcobox_set_parbox_indent_aux:n { \thetcolorboxnumber } }
  \let\tcb@set@parbox@indent\@empty
}

% re-initialize
\let\tcb@set@parbox@indent\tcb@set@parbox@indent@

\cs_new_protected:Npn \tcobox_set_parbox_indent_aux:n #1
  {
    \bool_lazy_and:nnT
      % if not inside a nested tcolorbox
      { \int_compare_p:nNn {#1} = { \thetcolorboxnumber } }
      % and if inside a "tcb@savebox" env (upper or lower part is non-empty),
      % hence not at the time of using saved \tcb@upperbox or \tcb@lowerbox
      % TODO: use a better check
      { \cs_if_exist_p:N \tcbbreak }
      {
        \OmitIndent
        % debug: typest a visual marker
        \llap{\smash{\rule{.2em}{1em}\textsuperscript{\thetcolorboxnumber}}}
      }
  }

\cs_generate_variant:Nn \hook_gput_next_code:nn { ne }
\cs_generate_variant:Nn \tcobox_set_parbox_indent_aux:n { o }
\ExplSyntaxOff
\makeatother

\tcbset{set parbox/.style={parbox=#1, title={\texttt{parbox=#1}}}}

\newcounter{ypos}
\newcommand{\testParboxFalse}[2][]{%
  \stepcounter{ypos}
  \begin{multicols}{2}
    \begin{tcolorbox}[set parbox=true,#1]
      #2
    \end{tcolorbox}%
    % \rlap{\rule{\columnwidth}{.5pt}}
    \RecordProperties{parbox=true\arabic{ypos}}{ypos}%
    \newcolumn
    \begin{tcolorbox}[set parbox=false,#1]
      #2
    \end{tcolorbox}%
    % \clap{\rule{2\columnwidth}{.5pt}}
    % two rules overlaps => "parbox=(true|false)" results in the same height
    \RecordProperties{parbox=false\arabic{ypos}}{ypos}%
  \end{multicols}
  \par
  % raise an error if heights of boxes with "parbox=(true|false)" are different
  \ifnum\RefProperty{parbox=true\arabic{ypos}}{ypos}=%
        \RefProperty{parbox=false\arabic{ypos}}{ypos}\relax
  \else
    \PackageError{debug}{Different heights!}{\detokenize{#2}}%
  \fi
}

\begin{document}

%% Emptyness

% All empty
\testParboxFalse[title=]{\tcblower}
\testParboxFalse{            \tcblower bar\par bar}
\testParboxFalse{foo\par foo \tcblower}
\testParboxFalse{foo\par foo \tcblower bar\par bar}

\newpage

%% Special beginning content(s)

% Content starts with a list
% issue #123, extra top space in "parbox=false" tcolorbox starting with a list
\testParboxFalse{
  \begin{enumerate}
    \item abc
  \end{enumerate}
  \tcblower
  \begin{enumerate}
    \item abc
  \end{enumerate}
}

% Content starts with another tcolorbox
\testParboxFalse{
  \begin{tcolorbox}
    nested\par foo
  \end{tcolorbox}
  \tcblower
  \begin{tcolorbox}[set parbox=false]
    nested\par bar
  \end{tcolorbox}
}

%\newpage

%% Misc special cases

% Content consists of nested tcolorbox(es)
% from issue #171, no "before skip" if nested in a "parbox=false" tcolorbox
\testParboxFalse{
  before
  \begin{tcolorbox}[before skip=20pt]
    test \texttt{before skip}
  \end{tcolorbox}
  after
  \tcblower
  before
  \begin{tcolorbox}[before skip=0pt]
    test \texttt{before skip}
  \end{tcolorbox}
  after
}

\end{document}

Current (v6.1.0) \OmitIndent unconditionally \OmitIndent conditionally
image image image
image image image

Example v2 typesets 7 pairs of examples

  • Current (v6.1.0): extra top space in 5th parbox=false example
  • \OmitIndent unconditionally: \OmitIndent mistakenly applied (marked by a vert followed by current box number in superscript) before boxes in 1st and 3rd parbox=false examples and inside nested parbox=true box in 6th.

@T-F-S
Copy link
Owner

T-F-S commented Jan 9, 2024

This is a very interesting idea. I tested your code and added some further tests. Currently, all seems to work with the conditional \OmitIndent. Thank you!

Even, if new things come up, the approach may be adaptable to further conditions. I will make further tests.

By the way: The height control test with \RecordProperties did not work for me. All ypos values are 0 in my .aux file...

@muzimuzhi
Copy link
Contributor Author

By the way: The height control test with \RecordProperties did not work for me. All ypos values are 0 in my .aux file...

I forgot to save positions first, (see texdoc ltproperties-doc, doc for predefined property xpos and ypos). :)

This \testParboxFalse should work. \UseName{tex_savepos:D}% added right before each \RecordProperties.

\newcommand{\testParboxFalse}[2][]{%
  \stepcounter{ypos}
  \begin{multicols}{2}
    \begin{tcolorbox}[set parbox=true,#1]
      #2
    \end{tcolorbox}%
    % \rlap{\rule{\columnwidth}{.5pt}}
    \UseName{tex_savepos:D}%
    \RecordProperties{parbox=true\arabic{ypos}}{ypos}%
    \newcolumn
    \begin{tcolorbox}[set parbox=false,#1]
      #2
    \end{tcolorbox}%
    % \clap{\rule{2\columnwidth}{.5pt}}
    % two rules overlaps => "parbox=(true|false)" results in the same height
    \UseName{tex_savepos:D}%
    \RecordProperties{parbox=false\arabic{ypos}}{ypos}%
  \end{multicols}
  \par
  % raise an error if heights of boxes with "parbox=(true|false)" are different
  \ifnum\RefProperty{parbox=true\arabic{ypos}}{ypos}=%
        \RefProperty{parbox=false\arabic{ypos}}{ypos}\relax
  \else
    \PackageError{debug}{Different heights!}{\detokenize{#2}}%
  \fi
}

@T-F-S
Copy link
Owner

T-F-S commented Jan 10, 2024

This \testParboxFalse should work. \UseName{tex_savepos:D}% added right before each \RecordProperties.

Yes, thank you. Works fine :-)

@T-F-S T-F-S added the bug label Jan 10, 2024
@T-F-S T-F-S self-assigned this Jan 10, 2024
@T-F-S
Copy link
Owner

T-F-S commented Jan 10, 2024

Fixed with https://github.com/T-F-S/tcolorbox/releases/tag/v6.2.0
@muzimuzhi Thanks again for the valuable input.

@T-F-S T-F-S closed this as completed Jan 10, 2024
@muzimuzhi
Copy link
Contributor Author

I'm still more or less worried about using \cs_if_exist_p:N \tcbbreak as a marker to check if it's inside a tcb@savebox environment (aka, if it's being saved to a title/upper/lower box). (\tcbbreak was chosen because I found no better markers in \tcb@lrbox.)

Would a new, specific boolean be needed?

@T-F-S
Copy link
Owner

T-F-S commented Jan 18, 2024

I'm still more or less worried about using \cs_if_exist_p:N \tcbbreak as a marker to check if it's inside a tcb@savebox environment (aka, if it's being saved to a title/upper/lower box). (\tcbbreak was chosen because I found no better markers in \tcb@lrbox.)

Would a new, specific boolean be needed?

A boolean marker will not enhance the code per se, but may prevent accidential code changes in future which may break the test. So, yes, I will add a boolean as marker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants