In issue #1815, it was reported that Gollum RSS feeds attempt to put
entire commit messages into a feed item's `<title>` element.
Given that commit messages can be many paragraphs long, this is not an
acceptable way to render a feed item title.
If a commit has many lines or paragraphs, we now put those inside the
`<description>` element as of this commit.
While I was editing this view, I decided it would be a good time to
increase test coverage, as this view renderer was not under test at
all. I also fixed a typo ("Commited" should say "Committed"). There are
implicit dependencies on other gems that provide the
`Gollum::Git::Commit`, `Gollum::Git::Actor`, and `Rugged::Commit`
classes here, so mocking out simple versions of their interfaces seemed
like the path of least resistance to setting up controllable tests.
I also removed some now-duplicated assertions from `test/test_app.rb`.
* Internationalize `Views::Compare` templates
* Internationalize `Views::Error` templates
* Internationalize `Views::History` templates
* Internationalize `Views::LatestChanges` templates
* Internationalize `Views::Layout` templates
* Internationalize `Views::Overview` templates
* Internationalize `Views::Search` templates
* Reset I18n load path after I18n helper tests
* Create locale helper for global translations
There are some translation strings we should use across multiple views.
Instead of duplicating the translations, we can use a different locale
helper method, `#tt`, to get a hash of all available translations.
Then, in a partial view like `pagination.mustache`, we can render
translations regardless of what the current view class is.
This commit adds the necessary helper, tests, and uses the new method to
render translations on the `pagination.mustache` template, which is used
by many other view classes (latest changes, history, and search).
* Start using `yarn` for vendor assets
Until this commit, Gollum has included JavaScript assets by committing
the source code to the `lib/gollum/public/gollum/javascript/` directory
and including them for compilation in the Sprockets asset manifest file
at `lib/gollum/public/gollum/javascript/app.js`. This has been a
reasonable way to deal with third-party JavaScript, but there are a few
downsides:
- It's a burden to find out if JavaScript dependencies have been
updated and require updating in Gollum.
- It doesn't give us good visibility into the JavaScript dependencies
required by our dependencies.
- It forces us to commit external code to our repository, which can
make our developer tools more difficult to configure or use. For
example: when I search for key words in the repository using
Ripgrep, I often get "garbage" results from minified JavaScript.
Managing JavaScript dependencies via a JS package manager can resolve
all of these issues.
This commit allows us to manage JavaScript dependencies using the Yarn
package manager for Node JS modules[1]. I chose Yarn over NPM for one
reason: Yarn is the JavaScript package manager that Rails uses by
default. So many Ruby developers will already be familiar with Yarn.
To demonstrate how this can change how we manage JS assets in Gollum,
I've configured Yarn and started to manage the `mousetrap` dependency
with it. I chose `mousetrap` to start because it's a smaller, mostly
uncomplicated dependency. I was easily able to manually test that
`mousetrap` is still working after re-compiling the assets.
Hopefully this gives anyone reading enough context to jump in and start
moving our third-party JS assets out of the codebase.
[1]: https://yarnpkg.com/
* Recompile assets
* Add dev environment setup info to CONTRIBUTING
Now that we require additional tooling to manage JavaScript
dependencies, it seemed reasonable to add more documentation around
setting up one's development environment.
* Don't compile assets without `yarn`-managed ones
If a developer compiled assets without first running `yarn install`, we
would get incomplete collection of up-to-date assets.
Let's ensure that the developer has all the required assets by enforcing
this in the precompile task they should be using to compile production
assets to begin with.
* Use `Minitest::Test`
`Test::Unit` is deprecated, and we can switch to `Minitest::Test` with
almost no side effects.
This commit does all of the work required to make Minitest tests run
with Gollum's existing test helpers.
* Change Minitest output format
- The `DefaultReporter` seems to have cleaner output than what we had
before.
- `color: true` ensures things are colorized. It's pretty nice.
* Tweak test formatting; fix order-dependent failure
The order-dependent failure has been been commented in code.
After manually bisecting the test suite, I was able to determine that
the template cascade tests were leaving the `@@template_priority_path`
set to an overridden value in tests run afterward.
* Tweak setting initialization
I could not see a meaningful reason behind calling `forbid` so early in
the settings initialization block. But moving the `forbid` call until
later resolved this issue.
In the real world, I don't see how this would cause issues. But I found
that calling `forbid` when `wiki_options[:allow_editing]` was set to
false caused order-dependent test errors where Sprockets would end up
being badly configured and left without an initialized
`Sprockets::Environment` object, which is required by the
`sprockets-helpers` gem to resolve asset paths.
The test that would cause errors is also in this commit diff. I've
updated it to be a bit more readable.
I also took this opportunity to review and clean up the `@allow_editing`
assignment, which seemed a bit obfuscated.
* Update migration script to test run warnings
When running the entire test suite, many warnings would be output:
/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: already initialized constant HYPHENATE
/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: previous definition of HYPHENATE was here
/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: already initialized constant PAGE_FILE_DIR
/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: previous definition of PAGE_FILE_DIR was here
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:91: warning: already initialized constant REPO
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:91: warning: previous definition of REPO was here
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:236: warning: already initialized constant TREE
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:236: warning: previous definition of TREE was here
/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: already initialized constant PAGE_FILE_DIR
/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: previous definition of PAGE_FILE_DIR was here
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:91: warning: already initialized constant REPO
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:91: warning: previous definition of REPO was here
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:236: warning: already initialized constant TREE
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:236: warning: previous definition of TREE was here
While it's unlikely that end users would ever see these warnings, as
they'd only be running the migration script once, they will always be
shown in our local test runs and CI run output.
So instead of using constants in our migration script, I change the
script to use class variables instead. This should not effect the
functionality of the migration script whatsoever.
* Use `File.exist?` instead of `File.exists?`
In Ruby 3.x, `File.exists?` is deprecated and outputs a warning.
* Improve "allow editing" tests
While making changes to the test suite, I ran into some issues with
these tests failing on occasion.
I added some setup and teardown to help with this.
But one thing I did notice is that the word "Upload" appears in the
response body whether uploading is enabled or not, so I made these
assertions more specific to the HTML rather than other places the word
"Upload" might appear (which is related to Critic Markup).
* Do not attempt to modify frozen strings
Using `#gsub!` here now results in an error. I am not entirely sure why
this hasn't happened before, but the error is straightforward:
FrozenError: can't modify frozen String: "Author %{author} is from %{location}"
/home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:45:in `gsub!'
/home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:45:in `fill_argument_content'
/home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:37:in `block in autofill'
/home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `each'
/home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `map'
/home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `autofill'
/home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:35:in `block in autofill'
/home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `each'
/home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `map'
/home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `autofill'
/home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:21:in `t'
/home/runner/work/gollum/gollum/test/gollum/views/test_locale_helper.rb:95:in `block (4 levels) in <top (required)>'
`#gsub!` attempts to modify a string in-place, which does not work when
a string is frozen. It seems that in some Ruby environments, strings
from YAML files are frozen.
* Fix another order-dependent test failure
Very occasionally, this test fails due to `Precious::App` settings set
in previous tests. You can reproduce this failure using this seed:
bundle exec rake TESTOPTS="--seed=42898"
* Add `.header-title` style
This style shows smaller text on mobile devices. It reverts back to the
`2em` size used in Primer for H1 tags.
* Use `.header-title` for all `#head` headings
This commit:
1. Updates all main templates to use the new `.header-title` class,
making these titles less disruptively large on mobile devices.
2. Centers these titles for mobile breakpoints only, re-setting them
back to left-aligned text for `md-` breakpoints and up.
Note that in `lib/gollum/templates/wiki_contentf.mustache`, the
`.header-title` is not a `#head .header-title`, so it doesn't inherit
all of the new styles added in this commit.
* Adjust padding and presentation of mobile menu
To give mobile page headers more structure, this commit adjusts the
padding to be more symmetrical. It also adds a `border-bottom` on mobile
only. In my opinion this improves readability by telling the reader,
"This is where the navigation menu ends and the page content begins."
* Recompile assets
* Fix failing spec
My changes to the header layout seem to changed where the spaces in
`last_response.body` are placed. So let's assert only that the content
is present, since that's all this test should actually care about.
* Add `i18n` dependency
We will use `i18n` to provide localization for Gollum's frontend. I
chose this because it's a well-supported, pretty normal Ruby library.
* Configure I18n
- Locale files will be kept in `lib/gollum/locales/[lang].yml`
- The available locales, to start, will be English (`en`).
* Add I18n interface for mustache templates
This commit adds an interface that allows mustache templates to get I18n
translation strings, transform any arguments that may be present in
them, and then render them on the frontend.
This is our first real step to getting internationalizing the Gollum
frontend.
An issue was reported (see #1721) where, in docker containers where the
`LANG` was not being set, `Precious::App` would serve Mustache templates
in an ASCII encoding. This caused templates to error out.
In the past, this would have happened in environments where `LANG` was
not set and the Gollum applciation configuration had enabled MathJax.
Now, it happens even if MathJax is disabled because of a UTF-8 character
I added to the mobile navigation menu.
Basically, we should enforce a UTF-8 encoding from now on to avoid
runtime errors related to ASCII encoding.
I made a mistake when I made `#title` a private method. I did not see
that it was being called from `layout.mustache` to generate the
`<title></title>` tag in the `<head></head>` of each page.
This fixes my error, and adds tests so the behaviour is more explicit.
* Fix tabnav styles on #create and #edit views
The Primer CSS-provided `tabnav` styles were not being used on the Edit
and Preview tabs on the create and edit pages.
After following Primer's documentation [1], it looks like we were using
the `aria-current` attribute incorrectly. Dynamically adding/removing
this attribute on the selected tab fixes the issue.
[1]: https://primer-css-git-next-inputs.primer.now.sh/css/components/navigation#tabnav
* Recompile static assets
It is more semantic to use a `<button>` tag in the place of an `<a>`
when there is no other page being linked to. In this circumstance, we're
using JavaScript to present a modal to the user on click.
This change makes the "Upload" and "Rename" buttons appear in the
browser's tab index.
There was a display issue, where navbar items's outline styles were
being cut off due to the parent `<nav>` element's margin and padding.
Fortunately, we can do away with the navbar wrapper div entirely. It was
not doing anything important except defining its children as
`TableObject` items. But we can just do this on the `<nav>` itself.
If the user has edit permissions, the "History" button is shown in a
button group with "Edit" and "Rename". If the user does not have edit
permissions, it's shown by itself.
This commit also renames the label from "Page History" to "History".
* Change footer and footer container spacing
This commit:
1. Removes the Primer `my-md-0` spacing from the `#wiki-footer`
container.
This gives it margins along the Y axis, spacing it out from
the sidebar and main wiki content containers on `md` and larger
screens.
2. Adds `px-4` padding to the `#footer-content`. which makes the
This makes the footer content inset the same way that sidebar
content is. This makes the content look more uniform on mobile
devices, when the sidebar and footer are presented one before
the other.
* Change spacing behaviour of `#wiki-sidebar`
To make `#wiki-sidebar` and `#wiki-footer` more similar, we can make
sure `px-4` padding is applied to the `-content` container, rather than
the outer container.