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

add warning blockquote style, carpentries/styles#49 #475

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

jsta
Copy link
Contributor

@jsta jsta commented Jul 19, 2020

This PR adds a blockquote style for warnings or errors, see #49.

See https://carpentries.github.io/lesson-example/04-formatting/index.html#special-blockquotes for a comparison against other blockquote styles. See https://getbootstrap.com/docs/3.3/components/ for a gallery of alternative icons.

Screenshot from 2020-07-19 14-33-25

If this looks good to people, I've prepared a PR to the lesson-example repository at: https://github.com/jsta/lesson-example/tree/warningcallout

@fmichonneau
Copy link
Contributor

This looks good. Thanks, @jsta!
I was wondering whether we should harmonize the style with the code warnings from here #455 but I think that the 2 use cases are distinct enough (callout warning in the text vs. warning generated by code) that it's not needed. What do @carpentries/lesson-infrastructure-committee members think?

@maxim-belkin
Copy link
Contributor

What is the motivation for adding this blockquote? What type of information would this blockquote contain?

@jsta
Copy link
Contributor Author

jsta commented Jul 20, 2020

For me, the motivation is to warn users about a change in the default setting of stringsAsFactors that breaks a lot of existing R lesson content. See datacarpentry/R-ecology-lesson#608 (comment) and datacarpentry/r-intro-geospatial#74 (comment)

@maxim-belkin
Copy link
Contributor

Thank you, Joseph. I think this type of information fits the description of the callout blockquote, so from that point of view we have this type of information already covered. But this is not to say that I'm strongly opposed to introducing this new blockquote. I would, however, like to see some other possible use cases for it so that we don't add something that works in a very specific case only.

@ErinBecker
Copy link
Contributor

I think that distinguishing between “helpful information” = callouts vs. “warning” = potential problems makes a lot of sense. For the SWC shell lesson, for example, I would divide the current callouts like this:

Callout

  • Slashes
  • Clearing your terminal
  • Manual pages on the web
  • Other hidden files
  • Orthogonality
  • Two more shortcuts
  • Sorting output
  • Two ways of doing the same thing
  • Good names for files and directories
  • Which editor
  • Control, Ctrl, or ^ Key (?)
  • Whats in a name
  • Wildcards
  • etc

Warning

  • Command not found
  • Home directory variation
  • Unsupported command line options
  • Deleting is forever
  • etc

Dividing things up this way will make it more clear to Instructors where “extra” information (currently in callouts) is necessary vs just helpful.

I might not use the label "warningerr" - maybe just "error"? Or "caution"?

@brownsarahm
Copy link
Contributor

I agree with Erin's distinction. Ways that we can support instructors differentiating through content will help a lot especially for new instructors who are getting used to the material/ format of teaching . etc.

for a label I like "caution" to use a distinct term for a human warning or common mistake from a compiler provided warning or error. Though we might use the "caution" to help people prevent compiler warnings and errors, having distinct terms will help in talking about things in the context of lesson development and maintainence. (after any learning curve to make people aware of the new feature).

@maxim-belkin
Copy link
Contributor

maxim-belkin commented Jul 23, 2020

Makes sense. I especially like the idea of using it for things like "Deleting is forever" in the Shell lesson.
.error and .warning are already taken but .caution and .alert are available, I believe

@jsta
Copy link
Contributor Author

jsta commented Jul 23, 2020

Good discussion here! I favor .alert and pushed this change to the PR. Happy to go with .caution if people prefer.

@maxim-belkin
Copy link
Contributor

@jsta, could you please squash the commits, add Fixes carpentries/styles#49 to the message body on a separate line and remove #49 from the title (or replace it with a full link carpentries/styles#49)? -- otherwise this issue will be attached to all issues 49 in all Carpentries lessons (that use this repository).

@maxim-belkin
Copy link
Contributor

and one last comment from me -- I think classes are sorted alphabetically, so if you're adding .alert put it first, and if people decide on .caution -- put it after .callout

@jsta jsta force-pushed the warningcallout branch from ad3ecf5 to 6c258af Compare July 23, 2020 20:58
@jsta jsta changed the title add warning blockquote style, #49 add warning blockquote style, carpentries/styles#49 Jul 23, 2020
@fmichonneau fmichonneau merged commit 2e9d38f into carpentries:gh-pages Jul 24, 2020
@fmichonneau
Copy link
Contributor

thank you everyone! I think this is going to be a very useful to the lesson template.

@maxim-belkin
Copy link
Contributor

🎉

I have to clarify one thing: Fixes user/repo#issue-number should be in the message body rather than title for it to automatically close an issue -- that's why it didn't work this time (I closed that issue manually).

zkamvar added a commit to zkamvar/styles that referenced this pull request Dec 22, 2020
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.

5 participants