Major revamp #3

chrisjsewell · 2020-05-01T04:13:56Z

No description provided.

chrisjsewell · 2020-05-02T23:44:53Z

@choldgraf I've done a major revamp of this, to more closely follow the bootstrap 4 system(s). Have a look at the docs and see what you think.

Note I'm using the RTD (non-bootstrap) theme here, because I wanted to check that it works for the AiiDA docs (the initial use case). But ideally we could make the docs for both this theme (non-bootstrap) and the pydata one (bootstrap), like nbsphinx does with their docs and multiple theme builds: https://nbsphinx.readthedocs.io

chrisjsewell · 2020-05-02T23:47:41Z

sphinx_panels/__init__.py

+    # TODO only load these is using a non-boostrap theme
+    app.add_css_file("bs-grids.css")
+    app.add_css_file("bs-cards.css")


Here I'd like to find a way to only load these CSS if bootstrap.min.css is not already being loaded (they just contain copied CSS from that, specific to each element)

I can't see any easy way to automate bootstrap theme discovery so, in e67f3ac, I have added a sphinx option to disable loading the bootstrap CSS

choldgraf

In general I think this looks really nice - see a few comments and questions in there

docs/index.rst

choldgraf · 2020-05-04T15:31:56Z

docs/index.rst


 .. code-block:: rst

    .. panels::
-        :container: container-lg pad-bottom-20
+        :container: container-lg pb-3


This doesn't always need :container: though, right? I feel like it should have reasonable defaults so users don't have to start writing their own bootstrap classes off the bat

No, the defaults are described in the lines above this, and are defined in the code here:

sphinx-panels/sphinx_panels/__init__.py

Lines 34 to 36 in 195e853

DEFAULT_CONTAINER = "container pb-4"

DEFAULT_COLUMN = "col-lg-6 col-md-6 col-sm-6 col-xs-12"

DEFAULT_CARD = "shadow"

Note that all classes I use in the documentation are built-in boostrap classes, which are included in the CSS loaded in this extension

choldgraf · 2020-05-04T15:33:33Z

docs/index.rst

+but it is advised you use the built-in bootstrap classes:
+
+- `Card colouring <https://getbootstrap.com/docs/4.0/utilities/colors/>`_  contextual classes: `bg-primary`, `bg-success`, `bg-info`, `bg-warning`, `bg-danger`, `bg-secondary`, `bg-dark` and `bg-light`.
+- `Padding and margins <https://getbootstrap.com/docs/4.0/utilities/spacing/>`_: `border-0`, `p-2`, `m-2`, ...


This makes me wonder if there'd be value in having something like "sphinx-bootstrap" that is just an easy way to define a few useful bootstrap elements with sphinx directives. You could imagine instructions like these being applied to a number of UI elements. (as opposed to having one sphinx extension per element type, such as "sphinx-panels", "sphinx-togglebutton", etc)

yep, if this existed it would be easy to add the dependency to this package. If you look at the CSS I have added, you'll see that I have divided into approximately the classes required for each aspect.

so there'd be something like a bootstrap meta-package that would make it possible for extension to depend on it and then those extensions don't have to worry about loading any bootstrap JS/CSS themselves?

That might be nice, then we could look into re-working sphinx-togglebutton so that it uses bootstrap, but without requiring users to use a theme that has bootstrap

Another thing that could benefit from bootstrap would be "question and answer" blocks: https://getbootstrap.com/docs/4.1/components/forms/

choldgraf · 2020-05-04T15:34:52Z

docs/index.rst

+    :footer: text-right
+
+    ---
+    column += p-1


It's unclear what's happening here - are you running your own python code inside of the panel content?

just using regexes to identify <column><+=><p-1>, then deciding whether to add to the existing classes, or reset them based on if = or +=. See:

sphinx-panels/sphinx_panels/__init__.py

Lines 134 to 139 in 195e853

if opt_match.group(2) == "+=":

classes[opt_match.group(1)] = (

classes.get(opt_match.group(1), []) + opt_match.group(3).split()

)

else:

classes[opt_match.group(1)] = opt_match.group(3).split()

setup.py

choldgraf · 2020-05-04T15:36:40Z

sphinx_panels/__init__.py

-                title = nodes.container(
-                    in_panel=True, classes=["card-title"] + classes["title"]
+            if "header" in data:
+                header = nodes.container(


one "gotcha" that we should make sure isn't an issue here is the word container. the pydata them realized that weird things were happening because container is a very common and specific bootstrap class, while Sphinx appends container to lots of things as well

I specifically account for this in:

sphinx-panels/sphinx_panels/__init__.py

Lines 302 to 307 in 195e853

def visit_container(self, node):

classes = "docutils container"

if node.get("in_panel", False):

# we don't want the CSS for container for these nodes

classes = "docutils"

self.body.append(self.starttag(node, "div", CLASS=classes))

chrisjsewell · 2020-05-04T15:56:54Z

Thanks for the comments @choldgraf, I'll look now. Note in 16794f4 I have just changed the default delimiters (see docs), so that they are less likely to clash with rST header underlines, but also added a config value to change them. Open to suggestions on what the defaults should be though.

Co-authored-by: Chris Holdgraf <choldgraf@berkeley.edu>

choldgraf · 2020-05-04T16:24:59Z

re: the delimiters, I'm guessing you've already considered and decided against just having another directive for these? E.g. following what sphinx-tabs does:

.. tabs::
   :tabs: config

   .. tab::
      :singletab: config

      tab1 content
   
   .. tab::
      :singletab: config

      tab2 content

   .. tab::
      :singletab: config

      tab3 content

?

chrisjsewell · 2020-05-04T16:51:57Z

re: the delimiters, I'm guessing you've already considered and decided against just having another directive for these?

Well yeh, I feel that (a) it adds the need for extra indentation (in rST at least), which I don't think is ideal and (b) it implies, assuming a panel directive, that it can be used outside the panels directive, and/or that there can be content in-between the panel directives, neither of which really make sense.

choldgraf · 2020-05-04T17:01:45Z

Yeah that makes sense - I think that the pros and cons of parsing panel structure/config with a single directive basically comes down to complexity of the panels. The more complex you want your panel structure to be (or, the more you want it to deviate from the default), the more clunky it'll feel to configure with special syntax that only applies to the sphinx-panels directive. The less-complex you want it to be, the more it'll be straightforward to just use --- delimiters or whatever.

chrisjsewell · 2020-05-04T17:12:17Z

The more complex you want your panel structure to be

But then you can just use the nested container directive approach that pandas-dev/pandas#32065 uses. In some respect, this is meant to be a short-hand for that, which is simpler to type and doesn't leave you with large amounts of nested directives in your source document.

choldgraf · 2020-05-04T17:13:16Z

@chrisjsewell that's a good point...at that point you're a "power user" so it's probably fine to expect folks to build their own containers etc

chrisjsewell · 2020-05-04T17:27:06Z

at that point you're a "power user"

Personally, as a "power user", I would still use this and feel it's a lot less clunky than having to use multiple directives. But yep, different strokes for different folks 😄

Ok cool well I think this is good to merge and release as v0.2.0. Unless you want to discuss anything else?

choldgraf · 2020-05-04T17:40:41Z

let's do it 🚀 - let's keep this "meta bootstrap" repository idea on the backburner and see if it could be useful in the future. I really like the idea of finding a way to expose bootstrap functionality to sphinx users without requiring them to use a particular theme

choldgraf · 2020-05-04T17:42:57Z

You know right after I pressed merge I had one other thought - rather than using a delimiter that's also got meaning in markdown/rST, what if we just used {panel} as a delimiter?

e.g.:

.. panels::

    Content of the top-left panel

    {panel}

    Content of the top-right panel

    {panel}

    Content of the bottom-left panel

    {panel}

    Content of the bottom-right panel

I feel like it is more likely somebody would want to write a line that starts with ... than a line that starts with {panel}, and in a way it is more verbose what that line means...

chrisjsewell · 2020-05-04T17:50:39Z

eurgh, dunno I prefer being able to use variable length delimiters. I think it's easier to see the delimiters quickly, if you had a lot of content in the panels, with:

.. panels::

    Content of the top-left panel

    ...................................................

    Content of the top-right panel

    ...................................................

    Content of the bottom-left panel

Happy to put it out for poll or something though lol

choldgraf · 2020-05-04T17:53:46Z

nah I think it's fine - I hadn't thought about variable-length. was just an idea to explore if you immediately jumped on it, but if your first thought is hesitation I think we should just go w/ the current approach and see if it raises problems over time

choldgraf · 2020-05-04T17:55:47Z

One other thought, then - the way that sphinx-gallery does this is to use two options for people. One is delimiters with #. e.g.:

Block 1

###################

Block 2

Which has the added benefit that it doesn't overlap with rST and also won't clash with ATX headers in markdown

Or another option which is to use Hydrogen and Pycharm-style delimiters like so:

Block 1

# %%

Blocks 2

I wonder if it would be good to piggy-back on one of those since there is already some prior art in the community for using those as delimiters?

Apologies for thinking of all this after hitting the big green button :-)

chrisjsewell · 2020-05-04T18:10:09Z

Using # as a delimiter would just be a matter of changing the default in:

sphinx-panels/sphinx_panels/__init__.py

Line 316 in b8d6b36

app.add_config_value("panels_delimiters", (".", "^", "+"), "env")

What is the link where you can do this in sphinx-gallery I can't actually find it

choldgraf · 2020-05-04T18:14:02Z

Here's where they describe how to structure sphinx-gallery python files:

https://sphinx-gallery.github.io/stable/syntax.html#embed-rst-in-your-example-python-files

They have a different use-case, which is to break up blocks of python into content blocks and code blocks, but perhaps the same idea could be used here?

chrisjsewell · 2020-05-04T18:19:47Z

ok right but (to state the obvious) thats for using rST in python files, which is not really the same use case. TBH I think there is enough flexibility in making the delimiters configurable. The only thing I would think of adding, is to allow the header and footer to be None to "switch them off", then you would only have one potential piece of syntax to clash with any existing syntax

Update README.md

1ad27cb

This was referenced May 1, 2020

Docs: scaffolding for the "Intro" section aiidateam/aiida-core#4011

Merged

Docs: Consider using CircleCI for documentation CI aiidateam/aiida-core#4028

Closed

chrisjsewell added 4 commits May 3, 2020 00:23

major update

b4ad94e

doc warning fixes

249904f

Add new CSS to distibution

a23cf86

uodate docs

1df6a59

chrisjsewell changed the title ~~Update docs~~ Major revamp May 2, 2020

chrisjsewell requested a review from choldgraf May 2, 2020 23:40

chrisjsewell commented May 2, 2020

View reviewed changes

This was linked to issues May 2, 2020

Allow for different numbers of columns #1

Closed

document arguments #4

Closed

vertically center all content #5

Closed

chrisjsewell added 7 commits May 3, 2020 00:52

typo

1f6f672

Add sphinx option for loading the bootstrap CSS

e67f3ac

improve parsing (+tests) and css

f00b0c4

minor css fix

7df6c41

add image caps

aac4ba5

update example

ecb8c5c

Update index.rst

ca61a9e

choldgraf reviewed May 4, 2020

View reviewed changes

chrisjsewell added 2 commits May 4, 2020 16:50

Changed delimiters and added configuration option for them

16794f4

fix tests

f20ba91

fix typo

195e853

Co-authored-by: Chris Holdgraf <choldgraf@berkeley.edu>

change names of config options

0a312cd

choldgraf merged commit b8d6b36 into master May 4, 2020

choldgraf deleted the testing2 branch May 4, 2020 17:40

	DEFAULT_CONTAINER = "container pb-4"
	DEFAULT_COLUMN = "col-lg-6 col-md-6 col-sm-6 col-xs-12"
	DEFAULT_CARD = "shadow"

	if opt_match.group(2) == "+=":
	classes[opt_match.group(1)] = (
	classes.get(opt_match.group(1), []) + opt_match.group(3).split()
	)
	else:
	classes[opt_match.group(1)] = opt_match.group(3).split()

	def visit_container(self, node):
	classes = "docutils container"
	if node.get("in_panel", False):
	# we don't want the CSS for container for these nodes
	classes = "docutils"
	self.body.append(self.starttag(node, "div", CLASS=classes))

Major revamp #3

Major revamp #3

Uh oh!

Conversation

chrisjsewell commented May 1, 2020

Uh oh!

chrisjsewell commented May 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

choldgraf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisjsewell May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisjsewell May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisjsewell commented May 4, 2020

Uh oh!

choldgraf commented May 4, 2020

Uh oh!

chrisjsewell commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

choldgraf commented May 4, 2020

Uh oh!

chrisjsewell commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

choldgraf commented May 4, 2020

Uh oh!

chrisjsewell commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

choldgraf commented May 4, 2020

Uh oh!

choldgraf commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisjsewell commented May 4, 2020

Uh oh!

choldgraf commented May 4, 2020

Uh oh!

choldgraf commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisjsewell commented May 4, 2020

Uh oh!

choldgraf commented May 4, 2020

Uh oh!

chrisjsewell commented May 4, 2020

Uh oh!

Uh oh!

chrisjsewell commented May 2, 2020 •

edited

Loading

chrisjsewell May 4, 2020 •

edited

Loading

chrisjsewell May 4, 2020 •

edited

Loading

chrisjsewell commented May 4, 2020 •

edited

Loading

chrisjsewell commented May 4, 2020 •

edited

Loading

chrisjsewell commented May 4, 2020 •

edited

Loading

choldgraf commented May 4, 2020 •

edited

Loading

choldgraf commented May 4, 2020 •

edited

Loading