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

Style visual test app views #1181

Merged
merged 3 commits into from
Jun 18, 2019
Merged

Conversation

ashmaroli
Copy link
Contributor

Previews

Index page

Index Page Sample)

Lexer Page

lApache lexer visual

/cc @jneen

@pyrmont pyrmont added the needs-review The PR needs to be reviewed label Jun 13, 2019
@ashmaroli
Copy link
Contributor Author

Rationale for using @raw and @comment_color

  • @raw is set to the resulting output from Rouge's Plaintext lexer, instead of existing CGI.escape_html(@sample) because the plaintext lexer output allows rendering line numbers for the sample text as well.
  • @comment_color ensures that attention is not drawn towards such elements allowing one to focus on highlighted spans better.

@pyrmont
Copy link
Contributor

pyrmont commented Jun 17, 2019

@ashmaroli Can you provide a bit more context to explain the purpose of this PR? Is it just to enhance readability? I'm not sure I see a large difference between your screenshots and how it looks now.

@pyrmont pyrmont added author-action The PR has been reviewed but action by the author is needed and removed needs-review The PR needs to be reviewed labels Jun 17, 2019
@ashmaroli
Copy link
Contributor Author

@pyrmont The purpose of this PR is to improve the look of the visual test views. There isn't a large difference between the views on master because I didn't want to disrupt the flow contributors and maintainers are used to, just improve it.

I shall list everything that's different with this PR:

  • index page has the various lexer-render samples flush with each other. There's a minor overhead to discern the boundaries between two lexer-render samples. The PR adds subtle borders to reduce this overhead.
  • /:lexer pages have a subtle improvements as well. The main headings are colored the same as the comment styles in current language. This is to improve focus on the main area: the body of sample text and the highlighted result.
  • The raw text has been changed to be the result of Rouge's plaintext lexer. This is to allow rendering line-numbers for the raw text as well. (This would help @miparnisari detect dropped lines by comparing the raw line numbers and highlight-render line numbers)
  • All pages will now have a footer with a Rendered with <theme> division. In future, I may add a couple of HTML <select> elements there to allow switching themes or other parameters to easily test various scenarios.

@pyrmont
Copy link
Contributor

pyrmont commented Jun 17, 2019

Line numbers for the raw sample sounds pretty sweet.

@pyrmont pyrmont added needs-review The PR needs to be reviewed and removed author-action The PR has been reviewed but action by the author is needed labels Jun 17, 2019
@pyrmont
Copy link
Contributor

pyrmont commented Jun 17, 2019

@ashmaroli I'd like to run this locally for a test a bit later when I'm at home. Should be able to merge when that's done.

@ashmaroli
Copy link
Contributor Author

I'd like to run this locally for a test

Of course. There's no hurry.

@pyrmont
Copy link
Contributor

pyrmont commented Jun 17, 2019

@ashmaroli Made some suggested fixes in ashmaroli#2 :)

@pyrmont
Copy link
Contributor

pyrmont commented Jun 18, 2019

@ashmaroli Did you have anything else you wanted to add? I think this can be merged in.

@pyrmont pyrmont merged commit c7c8ec3 into rouge-ruby:master Jun 18, 2019
@pyrmont pyrmont removed the needs-review The PR needs to be reviewed label Jun 18, 2019
@ashmaroli ashmaroli deleted the style-visual-test-app branch June 19, 2019 05:24
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.

None yet

2 participants