Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Improve the UI for the rules documentation #2275

Merged
merged 4 commits into from
Mar 3, 2017
Merged

Improve the UI for the rules documentation #2275

merged 4 commits into from
Mar 3, 2017

Conversation

ChrisMBarr
Copy link
Contributor

@ChrisMBarr ChrisMBarr commented Feb 28, 2017

PR checklist

  • Addresses an existing issue: #0000
  • enhancement
    • Includes tests
  • Documentation update

Overview of change:

This PR make some UI changes to the the list of rules, and the individual rule detail pages.

  • The rules on the rules list page are now clickable, not just the rule name
  • Alternating background colors + hover colors
  • "feature" badges for each rule in the list, and larger versions on the actual rule details page
    • yellow badge with document icon when typescriptOnly is true
    • green badge with wrench icon when hasFix is true
    • red badge with info icon when requiresTypeInfo is true

Here's what the changes look like!
tslint-docs

Eventually it would be really nice to have some interactive rule filters on that page. To be able to filter the rules list down to only show the rules that require type info, or could be auto-fixed would be awesome! I wanted to keep this PR minimal & focused though.

@ChrisMBarr
Copy link
Contributor Author

This shows that CircleCI is still running the tests, but clicking through to the details shows that they all passed! perhaps this was related to the Amazon S3 outage yesterday?

@nchen63
Copy link
Contributor

nchen63 commented Mar 1, 2017

For some reason, the info icon doesn't render for me (Chrome/OSX)

screen shot 2017-03-01 at 12 25 40 pm

@ChrisMBarr
Copy link
Contributor Author

ChrisMBarr commented Mar 2, 2017

@nchen63 Hmm, that's odd, it's just a standard unicode symbol. Here's a test page for that symbol, does it render here? http://www.fileformat.info/info/unicode/char/1f6c8/browsertest.htm

What about this one? This could be used instead and still have roughly the same meaning: http://www.fileformat.info/info/unicode/char/2139/browsertest.htm

If not, I think this could be just fine without any icons at all. The icons would only be a good addition if they work cross browser, we don't want those squares showing up!

@adidahiya
Copy link
Contributor

adidahiya commented Mar 2, 2017

@ChrisMBarr your first icon link doesn't work for me, but the second one does. I'm using OS X.

@ChrisMBarr
Copy link
Contributor Author

Ok, change made. Here's what it now should look like

image

@nchen63
Copy link
Contributor

nchen63 commented Mar 2, 2017

The circle is weirdly stretched out on my browser

screen shot 2017-03-02 at 3 52 26 pm

I pushed a commit to your branch that should fix the issue. Can you see if it looks ok on your side?

@ChrisMBarr
Copy link
Contributor Author

ChrisMBarr commented Mar 3, 2017

Ah good idea, that's a better solution to fix the width to 1em; to match the current font size. I had to move that up to apply to the regular sized badges as well, you added them to just the smaller ones that appeared in the rules list.

The larger ones appear on the actual rules page, and they looked like this:
2017-03-03 07_40_22-rule_ promise-function-async

But I've fixed that now!

I tested this on Windows 10 in Chrome, Firefox, Edge, IE11, IE10, and IE9 - all look good. Edge & all IE's render the icons a but different using the solid color non-emoji look to them, but they work.

I also added a whole bunch of comments in all the classes I added since I don't know who in the future might be looking at this code, nor do I know what they familiarity with SCSS/CSS is.

@nchen63 nchen63 merged commit ee721bb into palantir:master Mar 3, 2017
@nchen63
Copy link
Contributor

nchen63 commented Mar 3, 2017

@ChrisMBarr thanks!

@ChrisMBarr ChrisMBarr deleted the improve-docs branch March 3, 2017 17:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants