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

Cheat Sheet: Mention Definition Lists only or also good ol' table for references? #57

Closed
josefglatz opened this issue Feb 19, 2019 · 15 comments
Labels
decision required This issue requires a decision before we can proceed.

Comments

@josefglatz
Copy link

Yesterday I reviewed https://docs.typo3.org/typo3cms/HowToDocument/WritingReST/CheatSheet.html and I was thinking about the "table markup" which is used in many core reference manual sections like https://docs.typo3.org/typo3cms/TyposcriptReference/ContentObjects/CoaAndCoaInt/Index.html

image

I like the result of this approach. If you have many options like in TypoScript it is easier for me to scan those options until you find the right one. (my personal opinion!)

Using Definition Lists "nicely-styled"
image is a bit harder to consume (for me). It offers more possibilities.


My question are

  • Do we have to go with the newer Definition List only?
  • Do we have to improve the definition list or add another especially for such scenarios (option reference)?
  • Do we have to add the table based approach .. ### BEGIN~OF~TABLE ### also in this documentation?
  • Will the table based approach persist and should it be used by documentation authors?
@sypets
Copy link
Contributor

sypets commented Feb 19, 2019

Josef, I added 2 links to your post. I grepped the TypoScript reference and the only page using the definition list (with dl-parameters) is FLUIDTEMPLATE.

The second example (definition list) does convey more information: It includes default values and if the parameter is optional.

I agree, we should agree on one best practice and remove the other from "Writing Documentation".

@sypets sypets added the decision required This issue requires a decision before we can proceed. label Jul 2, 2019
@sypets
Copy link
Contributor

sypets commented Jul 2, 2019

To summarize: There are several directives in use for TypoScript reference:

  1. ..container:: table-row
.. container:: table-row


   Property
         if

   Data type
         :ref:`->if <if>`

   Description
         If "if" returns false, the COA is **not** rendered.

table-row

https://docs.typo3.org/m/typo3/reference-typoscript/master/en-us/ContentObjects/CoaAndCoaInt/Index.html

@sypets
Copy link
Contributor

sypets commented Jul 2, 2019

  1. definition list

definition-list

.. rst-class:: dl-parameters

   parameterAbc
      :sep:`|` :aspect:`Condition:` required
      :sep:`|` :aspect:`Type:` string
      :sep:`|` :aspect:`Default:` ''
      :sep:`|`

      Text describing parameterAbc ...

   parameterBcd
      :sep:`|` :aspect:`Condition:` optional
      :sep:`|` :aspect:`Type:` boolean
      :sep:`|` :aspect:`Default:` false
      :sep:`|`

      Text describing parameterBcd ...

https://docs.typo3.org/m/typo3/docs-how-to-document/master/en-us/WritingReST/DefinitionLists.html

@sypets
Copy link
Contributor

sypets commented Jul 2, 2019

  1. I also found another one:

.. container::ts-properties

I saw this in the Example Extension Manual:

.. container:: ts-properties

   =========================== ===================================== ======================= ====================
   Property                    Data type                             :ref:`t3tsref:stdwrap`  Default
   =========================== ===================================== ======================= ====================
   allWrap_                    :ref:`t3tsref:data-type-wrap`         yes                     :code:`<div>|</div>`
   `subst\_elementUid`_        :ref:`t3tsref:data-type-boolean`      no                      0
   wrapItemAndSub_             :ref:`t3tsref:data-type-wrap`
   =========================== ===================================== ======================= ====================

https://docs.typo3.org/m/typo3/guide-example-extension-manual/master/en-us/Configuration/Index.html#properties

@DanielSiepmann
Copy link
Member

DanielSiepmann commented Jul 2, 2019

We also have
image

:aspect:`Datatype`
    boolean

:aspect:`Scope`
    Display

:aspect:`Description`
    If set, then the :ref:`label_alt <ctrl-reference-label-alt>` fields
    are always shown in the title separated by comma.

    .. note::
        :ref:`label_userFunc <ctrl-reference-label-userfunc>` overrides this property.

https://docs.typo3.org/m/typo3/reference-tca/master/en-us/Ctrl/Index.html#label-alt-force

That one was preferred by Lolli, and therefore everything was migrated to this syntax on all manuals he reworked.

@marble
Copy link
Member

marble commented Jul 2, 2019

My take on this is:

  • I. Do not use (1.) ".. container:: table-row" That is a left over from the automatic conversion and often looks "broken" since some CSS classes are attached to special definition terms (DT) and that is neither complete and perfect nor understandable.

  • II. I very much promote the (2.) "definition list" solution. It is well writable in source, it's arbitrarily extensible und it produces a nice looking and "brain suitable" output.

  • III. lolli's objection to the (2.) isn't a general veto - it's only that he has invested a lot of work to write everything in the third form of that "label_alt_force" example as presented by Daniel in the isssue. He doesn't want to rework everything but doesn't want to have two styles in one manual either. My brain has much more difficulty to process the output of III. compared to that of II.

  • Note that all of the above three ways make use of the reST construct "definition list". The text-roles ':aspect:' and ':sep:' (=separator) are only there for styling purposes. Once you get that they are easy to write.

  • IV. There still my be cases where ordinary tables may be appropriate. But in general I wouldn't recommend that. They don't adapt to the various screen sizes that well.

My conclusion and advice:
Don't use (I.). If you can in terms of how much has to be reworked use (III.). If you're stuck on (II.) stick to that. Be consistent in your manual. Ordinary tables are not forbidden, but usually II. or III. are more what the reader is looking for.

@sypets
Copy link
Contributor

sypets commented Jul 2, 2019

To sum it up: I am completely confused now. Sorry, Martin. You explain things well.

I am undecided about what to do. Discussing about things like this takes away time that is needed desperately for other things.

On the other hand, using various constructs inconsistently makes things more difficult for contributors. People are still struggling with reST. If they come across new things on every corner, it doesn't make it easier. If we can decide on a smal subset of directives and ways to do things we can remove the unwanted directives from "Writing Documentation" and the manuals. Makes it easier for everyone.

So, is this really a problem? People usually extend what is already there.

If things are done consistently in one manual, that is already a good thing.

I think it would still be preferable to dumb things down (if that can be done without getting rid of useful features) and use a smaller subset. Use one thing consistently.

Also, if we can't even decide on one convention to do this or on how to indent ...


Current status quo:

  • TS ref uses ..container:: table-row more than 500 times. That is no longer to be used if I understand corrrectly.
grep -r "..container:: table-row" . | wc -l
502

  • TCA ref apparently uses method 4 (which Daniel brought up) but have not checked this.

@marble
Copy link
Member

marble commented Jul 4, 2019

Proposal of a solution

Sorry, I didn't want to screw you up. Just did the best to explain the current state, as you asked.
The good thing is: You gave me reason to explore and find a real solution, and I made some investigations yesterday. I think I have found a really good solution. It goes like this:

No text-roles anymore

We drop all those extra text-roles like 'aspect' and 'sep'. They shouldn't be in reST anyway as they were only introduced to create visual effects. The object we want to describe goes into a headline. The various "aspects" (= properties) are listed in a definition list. And the actual description is the normal text that belongs to this headline (=section content). Example:
image

So this is just plain reST, that should look reasonable on Github too. Something like this:
image

Make it a headline

Usually we need a headline for the object anyway, and we can add a label in the usual way to allow cross-referencing:
image
And we get the various advantages of headlines.

Now the trick

We add a single line right before the headline like:
image

Technically this creates a <div class="t3object section" id="languageid"> … </div> for the whole section which is enough to give it a special styling by css only. No Sphinx extension required.

Order of prefix lines is not significant

Works:
image

Works:
image

Final example

Source

image

Visual result (just a demo - not done yet)

We don't have that yet, as the css still needs to be created. It could look something like this:
image

Implications

Applying this idea would mostly mean "cleaning the code". Which, I would say, is good!

Benefits

Simplicity, flexibility, nice reading experience.

@sypets
Copy link
Contributor

sypets commented Feb 7, 2020

I think the proposal by Martin (in previous comment) is very good: It is simpler to write. You use generic markup. You don't have to add additional aspects or tables which are more tedious to write. How this is styled, can be decided in the theme, which is cleaner.

I created a new issue in the theme repo to address this: TYPO3-Documentation/sphinx_typo3_theme#48

@sypets
Copy link
Contributor

sypets commented Feb 7, 2020

From team meeting today: #57 (comment)

Proposal is to use confval directive. Please ignore previous comment.
Martin: Use confval directive: https://docs.typo3.org/m/typo3/demo-t3SphinxThemeRtd/master/en-us/Directives/confval.html
(is used in documentation of sphinx project)

Example

.. confval:: mr_pommeroy

   :default: Happy new year, Sophie!
   :type:    shy

   Participant of Miss Sophie's birthday party.

@linawolf
Copy link
Member

linawolf commented Jan 1, 2021

I tried this out with the Data Processors I am currently working on. The syntax is so much more clear and easy to adapt. I really love it!!

`
.. confval:: if

:Required: false
:type: :ref:if condition
:default: ""

If the condition is not met the data is not processed
`
opposed to:

`
.. rst-class:: dl-parameters

if
:sep:| :aspect:Required: false
:sep:| :aspect:Type: :ref:if condition
:sep:| :aspect:Default: ''
:sep:|

If the condition is not met the data is not processed
`
However I find the old design much more compact and nice then the new one. Even if the colors of the old one are some-what old fationed.

parameters_old
parameters_new

@linawolf
Copy link
Member

linawolf commented Jan 1, 2021

One other feature I am missing is, how do we create an automatic overview of properties like here:

https://docs.typo3.org/m/typo3/reference-typoscript/10.4/en-us/ContentObjects/Fluidtemplate/Index.html

.. contents:: :local: :depth: 1

doesn't seem to work with the directives:

@marble
Copy link
Member

marble commented Jan 3, 2021

Funfact (me playing stupid): @linawolf Did you know, that just by using the .. confval:: directive you are already automatically creating an overview about those items? It will automatically be part of the 'Index'. Examples:

Just to mention: It is even possible to not only have 'confval' meaning 'configuration value' but something else like 'tsconfval' with meaning 'TypoScript item' additonally, because the 'confval' directive is defined in our conf.py file. The definition is here: https://github.com/t3docs/docker-render-documentation/blob/master/ALL-for-build/Makedir/conf.py#L471 I had seen it somewhere and just made it available for us. It could easily be used to add another directive. That's all I know about its functionality.

@sypets
Copy link
Contributor

sypets commented Mar 13, 2021

@marble @linawolf Looking at this again after some time. Would you be so kind as to summarize the status quo now? Has this been resolved and decided or not? What is the syntax that is to be used now and can you link to example pages and add screenshots?

Has that been resolved, what Lina wrote in #57 (comment)

One other feature I am missing is, how do we create an automatic overview of properties like here:

What I find important is that we agree on and focus on what we want to achieve:

  1. Visually separate the properties (this works well with the boxes) - make the pages easily skimmable and see where one property starts and one ends
  2. Easy to edit and maintain
  3. Automatically create an overview of all properties (as with .. contents:: :local: :depth: 1)
  4. Make it possible to link to each property (anchor)
  5. ... more?

@linawolf
Copy link
Member

The confvals are documented in the reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
decision required This issue requires a decision before we can proceed.
Projects
None yet
Development

No branches or pull requests

5 participants