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

Enhance docs #109

Merged
merged 30 commits into from
Oct 29, 2024
Merged

Enhance docs #109

merged 30 commits into from
Oct 29, 2024

Conversation

jimjam-slam
Copy link
Collaborator

Have filled the docs out a bit more and done some light editing! Still a few things missing, but I think they're mostly connected to fixes and features we haven't implemented yet. What do you think, @andrewpbray?

@andrewpbray
Copy link
Contributor

This is excellent! As I look through your work, if I have small edits, I'll make them directly, but please feel free to object if you feel strongly!

docs/index.qmd Outdated
@@ -28,18 +31,20 @@ format: closeread-html

::::{.cr-section layout="overlay-center" style="font-size: 1.5em; background-color: rgba(39, 128, 227, 0.2);"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried setting the opacity to 1 here but I couldn't get it to override the sourceCode opacity that Quarto uses.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmmm, this was the background of the code block, right? Do we have that set in our theme already?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the background of the whole cr-section; oh right: I guess its the sticky that we want to have opacity 1 ... one sec...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, when I add inline styling to the sticky in the qmd it still doesn't take. In fact even editing through the browser inspector I can't get the opacity to take. How did you fix it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the heavy nesting of code blocks, you need to actually do it from a stylesheet rather than inline.

.cr-section {
  .sticky-col {
    .sticky-col-stack {
      div.sourceCode {
        background-color: rgba(233, 236, 239, 1);
      }
    }
  }
}

I'm a bit worried this might override user theming (I don't see it set as a Bootstrap var, but it might come through Quarto from a syntax processor), but let's see how we go. Have added this now :)

@andrewpbray
Copy link
Contributor

Before I implement this whole hog, I have a hot take to run by you: I'm not a fan of callout boxes 😬

I find my eye reads them first (because of the box, because of the color), before i get oriented to the hierarchy of the sections. Indeed they call my attention, but usually in a way that ruins my flow of parsing a page in a more linear fashion.

If you're strongly pro-callout, I'm totally fine keeping em in, but just thought I'd propose an edit :)

@jimjam-slam
Copy link
Collaborator Author

jimjam-slam commented Oct 25, 2024

Haha, no worries at all — I do like callouts generally, but I don't have strong opinions on them here.

I tend to also use them for parenthetical stuff, too, and that stuff (ie. anything marked.callout-note) could probably be chucked into the margin or under a footnote instead! As you say, that stuff should probably be de-emphasised!

Copy link
Contributor

@andrewpbray andrewpbray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!


Need help, or looking for others to connect with around Closeread? Come join us on the [discussion board](https://github.com/qmd-lab/closeread/discussions) or [file an issue](https://github.com/qmd-lab/closeread/issues).

:::{.callout-note collapse="true"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this down here since it seemed niche and cluttered up the above-the-fold bit.

@@ -71,5 +76,28 @@ Draw your readers attention with: @cr-features
\
\

For guidance on how to author closeread documents, read the [Guide](guide/index.qmd). For examples of closeread documents alongside their source, see the [Gallery](gallery/index.qmd). For a catalog of the syntax and yaml options used in the closeread extension, see the [Reference](reference/index.qmd).
## What's next?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole section is great!

docs/index.qmd Outdated
@@ -28,18 +31,20 @@ format: closeread-html

::::{.cr-section layout="overlay-center" style="font-size: 1.5em; background-color: rgba(39, 128, 227, 0.2);"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the background of the whole cr-section; oh right: I guess its the sticky that we want to have opacity 1 ... one sec...

docs/index.qmd Outdated
@@ -28,18 +31,20 @@ format: closeread-html

::::{.cr-section layout="overlay-center" style="font-size: 1.5em; background-color: rgba(39, 128, 227, 0.2);"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, when I add inline styling to the sticky in the qmd it still doesn't take. In fact even editing through the browser inspector I can't get the opacity to take. How did you fix it?


### Progress blocks {#progress-blocks}

If you want to animate a graphic across several triggers, you could manually work out arithmetic that uses both the trigger index and progress. But, due to the way our underlying libraries report progress, this can occasionally lead to hiccups - and the maths can be a little frustrating!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if first-time readers will be able to make the jump to what you mean by "manually work out the arithmetic". Maybe best to document just the variables and progress block syntax here and refer them to the OJS demo for an example of two ways to implement it?

:::{.progress-block}
Check out our graphic @cr-graphic

We can keep referring to it as several triggers scroll by... @cr-graphic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe it's necessary to repeat the triggers here, is it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I did in my talk because... actually I can't remember why!

docs/guide/styling.qmd Show resolved Hide resolved
@jimjam-slam jimjam-slam mentioned this pull request Oct 28, 2024
@jimjam-slam jimjam-slam merged commit f047262 into dev Oct 29, 2024
1 check passed
@jimjam-slam jimjam-slam deleted the jimjam-slam/issue77 branch October 29, 2024 23:04
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.

2 participants