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

use sans-serif font most of the time #97

Merged
merged 4 commits into from
Mar 22, 2022
Merged

use sans-serif font most of the time #97

merged 4 commits into from
Mar 22, 2022

Conversation

davidjgoss
Copy link
Contributor

@davidjgoss davidjgoss commented Mar 15, 2022

🤔 What's changed?

Changed the font for our React components to sans serif (copied from Tailwind's standard sans-serif stack) except:

  • Error messages (and stack traces)
  • Doc strings
  • Text attachments

⚡️ What's your motivation?

Fixes #84.

🏷️ What kind of change is this?

  • 🐛 Bug fix (non-breaking change which fixes a defect)

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

Copy link
Contributor

@aurelien-reeves aurelien-reeves left a comment

Choose a reason for hiding this comment

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

It looks like scenario descriptions still uses monospace font:

image

Is this expected?

@davidjgoss
Copy link
Contributor Author

@aurelien-reeves this took me a while to figure out!

Here's the feature from the CCK:

Feature: Attachments
  It is sometimes useful to take a screenshot while a scenario runs.
  Or capture some logs.

  Cucumber lets you `attach` arbitrary files during execution, and you can
  specify a content type for the contents.

  Formatters can then render these attachments in reports.

  Attachments must have a body and a content type

  Scenario: Strings can be attached with a media type
    Beware that some formatters such as @cucumber/react use the media type
    to determine how to display an attachment.

    When the string "hello" is attached as "application/octet-stream"

From debugging, it seems the leading whitespace in the descriptions is preserved when Gherkin parses, so each line of the scenario description has four leading spaces. We then treat it as Markdown, which interprets the four leading spaces as a code block:

image

Is this something we need to fix in Gherkin i.e. replicate the same behaviour as doc strings for normalising indentation?

@aurelien-reeves
Copy link
Contributor

Wow 🤯

I would say that we may have to fix this in gherkin. I see any reason why those spaces are kept in the description.

However, @aslakhellesoy may certainly have way more context than I have 😅

@aurelien-reeves
Copy link
Contributor

In the meantime, we may consider that PR done and treat that "issue" apart, whatever we decide it is an issue in gherkin, or we should treat it in react

@davidjgoss davidjgoss merged commit 8d72b9a into main Mar 22, 2022
@davidjgoss davidjgoss deleted the font-sans-mono branch March 22, 2022 14:36
@mattwynne
Copy link
Member

Agree we should fix this in Gherkin.

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.

HTML generated from the plugin has been changed to use monospaced fonts for explanatory text
3 participants