2c03a906de
The goal of this redesign is to provide two clear concepts that are
treated as separate concerns:
1. Mapping of specific key bindings to a semantic shortcut
2. Mapping of a semantic shortcut to its behavior
Previously, there was no notion of a semantic shortcut. This required
manual crafting of the help dialog and made it difficult to find an
approach to customizing key bindings.
Although supporting key binding customization is outside the scope of
this change, the path forward is more clear. Currently key bindings are
hard-coded during the creation phase of the gr-app element. It's
conceivable that a plugin or user settings feature could override these
bindings at runtime. Or they might be read from some server-side config
and passed down to gr-app via the index.html template.
The universe of possible shortcuts is now defined in an enum by
keyboard-shortcut-behavior. This is also where all the help text is
declared and organized.
The keyboard shortcut help dialog is now a generic container that reads
the current state of actively bound shortcuts and their help content
from keyboard-shortcut-behavior.
The set of declared key bindings that are active (and registered with
suitable listeners on the body element) is determined by monitoring the
attachment and detachment of elements that mix in
keyboard-shortcut-behavior and declare a keyBindings() method. This
method replaces the keyBindings object property that
iron-a11y-keys-behavior looks for. The method should return an object
mapping Shortcut enum values to handler names.
Elements that implement a shortcut behavior but only conditionally
attach to the DOM must depend on a container element to declare the
shortcut and pass the event down. An example is SAVE_COMMENT. The
binding for this shortcut should be active on change and diff views
regardless of whether any comments exist, so that it will be listed in
the help dialog.
The result of this change produces behavior that is close to previous
behavior, but not quite identical. Shortcuts that used to be listed
under a "Change list" or "Dashboard" section header now appear as
"Actions." The exact ordering of shortcuts has not been preserved, either.
Change-Id: Ib3fe44d502a83f7012e30615fbea9da2ab112eb7
(cherry picked from commit 5cde0849d4)
PolyGerrit
Installing Bazel
Follow the instructions here to get and install Bazel.
Installing Node.js and npm packages
The minimum nodejs version supported is 8.x+
# Debian experimental
sudo apt-get install nodejs-legacy
sudo apt-get install npm
# OS X with Homebrew
brew install node
brew install npm
All other platforms: download from nodejs.org.
Various steps below require installing additional npm packages. The full list of dependencies can be installed with:
sudo npm install -g \
eslint \
eslint-config-google \
eslint-plugin-html \
typescript \
fried-twinkie \
polylint \
web-component-tester
It may complain about a missing typescript@2.3.4 peer dependency, which is
harmless.
If you're interested in the details, keep reading.
Local UI, Production Data
This is a quick and easy way to test your local changes against real data. Unfortunately, you can't sign in, so testing certain features will require you to use the "test data" technique described below.
Running the server
To test the local UI against gerrit-review.googlesource.com:
./run-server.sh
Then visit http://localhost:8081
Local UI, Test Data
One-time setup:
- Build Gerrit
- Set up a local test site. Docs here and here.
When your project is set up and works using the classic UI, run a test server that serves PolyGerrit:
bazel build polygerrit &&
$(bazel info output_base)/external/local_jdk/bin/java \
-jar bazel-bin/polygerrit.war daemon --polygerrit-dev \
-d ../gerrit_testsite --console-log --show-stack-trace
Serving plugins
Local dev plugins must be put inside of gerrit/plugins
Loading a single plugin file:
./run-server.sh --plugins=plugins/my_plugin/static/my_plugin.js
Loading multiple plugin files:
./run-server.sh --plugins=plugins/my_plugin/static/my_plugin.js,plugins/my_plugin/static/my_plugin.html
Running Tests
This step requires the web-component-tester npm module.
Note: it may be necessary to add the options --unsafe-perm=true --allow-root
to the npm install command to avoid file permission errors.
Run all web tests:
./polygerrit-ui/app/run_test.sh
To allow the tests to run in Safari:
- In the Advanced preferences tab, check "Show Develop menu in menu bar".
- In the Develop menu, enable the "Allow Remote Automation" option.
If you need to pass additional arguments to wct:
WCT_ARGS='-p --some-flag="foo bar"' ./polygerrit-ui/app/run_test.sh
For interactively working on a single test file, do the following:
./polygerrit-ui/run-server.sh
Then visit http://localhost:8081/elements/foo/bar_test.html
To run Chrome tests in headless mode:
WCT_HEADLESS_MODE=1 ./polygerrit-ui/app/run_test.sh
Toolchain requirements for headless mode:
- Chrome: 59+
- web-component-tester: v6.5.0+
Style guide
We follow the Google JavaScript Style Guide with a few exceptions. When in doubt, remain consistent with the code around you.
In addition, we encourage the use of ESLint. It is available as a command line utility, as well as a plugin for most editors and IDEs.
eslint-config-google is a port of the Google JS Style Guide to an ESLint
config module, and eslint-plugin-html allows ESLint to lint scripts inside
HTML.
We have an .eslintrc.json config file in the polygerrit-ui/ directory configured
to enforce the preferred style of the PolyGerrit project.
After installing, you can use eslint on any new file you create.
In addition, you can supply the --fix flag to apply some suggested fixes for
simple style issues.
If you modify JS inside of <script> tags, like for test suites, you may have
to supply the --ext .html flag.
Some useful commands:
- To run ESLint on the whole app, less some dependency code:
eslint --ignore-pattern 'bower_components/' --ignore-pattern 'gr-linked-text' --ignore-pattern 'scripts/vendor' --ext .html,.js polygerrit-ui/app - To run ESLint on just the subdirectory you modified:
eslint --ext .html,.js polygerrit-ui/app/$YOUR_DIR_HERE - To run the linter on all of your local changes:
git diff --name-only master | xargs eslint --ext .html,.js
We also use the polylint tool to lint use of Polymer. To install polylint,
execute the following command.
To run polylint, execute the following command.
bazel test //polygerrit-ui/app:polylint_test
Template Type Safety
Polymer elements are not type checked against the element definition, making it trivial to break the display when refactoring or moving code. We now run additional tests to help ensure that template types are checked.
A few notes to ensure that these tests pass
- Any functions with optional parameters will need closure annotations.
- Any Polymer parameters that are nullable or can be multiple types (other than the one explicitly delared) will need type annotations.
These tests require the typescript and fried-twinkie npm packages.
To run on all files, execute the following command:
./polygerrit-ui/app/run_template_test.sh
To run on a specific top level directory (ex: change-list)
TEMPLATE_NO_DEFAULT=true ./polygerrit-ui/app/run_template_test.sh //polygerrit-ui/app:template_test_change-list
To run on a specific file (ex: gr-change-list-view), execute the following command:
TEMPLATE_NO_DEFAULT=true ./polygerrit-ui/app/run_template_test.sh //polygerrit-ui/app:template_test_<TOP_LEVEL_DIRECTORY> --test_arg=<VIEW_NAME>
TEMPLATE_NO_DEFAULT=true ./polygerrit-ui/app/run_template_test.sh //polygerrit-ui/app:template_test_change-list --test_arg=gr-change-list-view