Skip to content
This repository was archived by the owner on Nov 20, 2024. It is now read-only.

Updated doc gen for proposed new site design #2114

Merged
merged 5 commits into from
Jun 29, 2020

Conversation

parlough
Copy link
Collaborator

@parlough parlough commented May 28, 2020

Depended on by: #2115

Read the linked PR for more information, but this implements some minor cleanup and organizational changes for the proposed new site design.

As I mentioned on the other PR, please feel free to to present heavy feedback and large changes.

@parlough parlough marked this pull request as draft May 28, 2020 23:23
Copy link
Contributor

@pq pq left a comment

Choose a reason for hiding this comment

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

In general this looks great but we should discuss what to me feels like a bit of an emphasis change, directing a lot of attention to the style guide which is only part of what the linter has evolved to support (e.g., vs. flutter, pedantic, and internal-only rules).

Is this a conscious redirection? If so, will the style guide evolve to host or otherwise promote docs and guidance for these different styles?

@@ -268,7 +271,8 @@ class Generator {
sb.writeln('<p>');
sb.write('Incompatible with: ');
var rule = incompatibleRules.first;
sb.write('<a href = "$rule.html" >$rule</a>');
sb.write(
'<!--suppress HtmlUnknownTarget --><a href = "$rule.html" >$rule</a>');
Copy link
Contributor

Choose a reason for hiding this comment

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

💎

@parlough
Copy link
Collaborator Author

@pq You make a good point. When I was making these changes, it was more from a consistency standpoint to include the style button on more pages.

Perhaps we can remove this from the top bar and include it in a similar place to other guidelines. Then the top bar buttons could be reserved solely for the list of rules and if desired something else like a usage guide page or something else you have in mind.

@pq
Copy link
Contributor

pq commented Jun 1, 2020

Perhaps we can remove this from the top bar and include it in a similar place to other guidelines.

SGTM. Thanks!

On a related note, it occurs to me that we had issues with navigation on mobile. (See dart-lang/sdk#57856.) I wonder: does this arc of work fix that? (Or could you take a look perhaps while you're at it?)

Thanks a million for taking all of this on!

@parlough
Copy link
Collaborator Author

parlough commented Jun 28, 2020

@pq I went ahead and kept information about the style good near other rule sets and opted to include a link to more information about using the Linter.

As for being more mobile friendly, the buttons fit on a lot of screen sizes, but when they do not you get something like this:
image

This seemed to be the intention of the original, but it didn't really work.

@pq
Copy link
Contributor

pq commented Jun 29, 2020

Beautiful. Are we ready to land?

Thanks so much!

Copy link
Contributor

@pq pq left a comment

Choose a reason for hiding this comment

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

Thank you!

👍

@parlough
Copy link
Collaborator Author

Beautiful. Are we ready to land?

Thanks so much!

If you're happy with all of the changes, the PRs are good from me :). Can always make further changes and cleanup later.

@pq
Copy link
Contributor

pq commented Jun 29, 2020

Excellent. I'll merge and push a new version of the docs. Thanks!

Could you give me attribution info for https://github.com/dart-lang/linter/blob/master/AUTHORS?

@pq pq marked this pull request as ready for review June 29, 2020 17:00
@pq pq merged commit 0bdf0d4 into dart-archive:master Jun 29, 2020
nscobie pushed a commit to nscobie/linter that referenced this pull request Aug 2, 2020
* Update document generation for site changes

* Remove extra view ports

* Fix effective Dart link

* Format doc tool

* Add tooltip to version, rework top bar buttons, and other cleanup
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

3 participants