forked from ringcentral/slate
-
Notifications
You must be signed in to change notification settings - Fork 181
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
Static table of contents #701
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Still need to do: - put in old ToC styling and animations - fix weird bugs with Error header not showing up...I suspect the nesting code is incorrect
…des wouldn't appear in ToC
adamchainz
added a commit
to adamchainz/slate_apm_help
that referenced
this pull request
Jul 4, 2019
For files that git log'd back to the last Slate upgrade in scoutapp#4 / 63f4722 , I picked the upstream versions. In others I tried to sensibly merge, for example the README is all about Scout so I didn't take anything from upstream, whilst for the Gemfile there was only minimal Scout customization so I made the two more similar. The upstream ToC behaviour changed a lot in slatedocs#701, and Scout's had diverged with Derek's fix to scrolling in a8f2b7c. I took the upstream. This meant removing the ToC style overrides in `screen_overrides.css.scss` because the div has changed. `layout.erb` had obviously been customized but I think I managed to correctly merge. In `_variables.scss`, some changes affect the design: * `$nav-active-parent-bg` - This was added upstream as a grey related to `$nav-active-bg`; I did the same here and picked a grey related to the custom blue Scout uses for `$nav-active-bg` by reducing its saturation and value in GIMP. * `$nav-active-parent-text` is new, but it and its peers are all white, so I left it as-is. * `$nav-active-shadow`, `$nav-embossed-border-top`, `$nav-embossed-border-bottom`, and `$main-embossed-text-shadow` were removed. Notably the embossing was removed in slatedocs@5d0bef6#diff-f9212fb5aed21de00ff3495a78afb71d for a "more 2017 look." Could add that back if you want to preserve the style, though Scout had removed some of the text shadows already in 9c89ce3. I tested with `vagrant destroy` then `vagrant up`, and checked it out in my browser. Before/after screenshots attached to PR. Also during startup there was this error during `bundle install`: ``` default: middleman-core (4.2.1) has dependency bundler (~> 1.1), which is unsatisfied by the current bundler version 2.0.2, so the dependency is being ignored default: Following files may not be writable, so sudo is needed: default: /usr/local/bin default: /var/lib/gems/2.4.0 default: /var/lib/gems/2.4.0/build_info default: /var/lib/gems/2.4.0/cache default: /var/lib/gems/2.4.0/doc default: /var/lib/gems/2.4.0/extensions default: /var/lib/gems/2.4.0/gems default: /var/lib/gems/2.4.0/specifications ``` However I think this is fine, bundler is installed as a newer version so that's okay?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Section separators can come in a later PR — I think the right time to solve that is at the same time as solving the multiple-HTML-pages-per-ToC issue.