diff options
Diffstat (limited to 'doc/development/fe_guide')
-rw-r--r-- | doc/development/fe_guide/accessibility.md | 364 | ||||
-rw-r--r-- | doc/development/fe_guide/dependencies.md | 2 | ||||
-rw-r--r-- | doc/development/fe_guide/design_anti_patterns.md | 4 | ||||
-rw-r--r-- | doc/development/fe_guide/graphql.md | 135 | ||||
-rw-r--r-- | doc/development/fe_guide/icons.md | 6 | ||||
-rw-r--r-- | doc/development/fe_guide/img/editor_lite_create_ext.png | bin | 81777 -> 16495 bytes | |||
-rw-r--r-- | doc/development/fe_guide/index.md | 2 | ||||
-rw-r--r-- | doc/development/fe_guide/performance.md | 14 | ||||
-rw-r--r-- | doc/development/fe_guide/principles.md | 2 | ||||
-rw-r--r-- | doc/development/fe_guide/security.md | 4 | ||||
-rw-r--r-- | doc/development/fe_guide/style/javascript.md | 21 | ||||
-rw-r--r-- | doc/development/fe_guide/style/vue.md | 6 | ||||
-rw-r--r-- | doc/development/fe_guide/troubleshooting.md | 26 | ||||
-rw-r--r-- | doc/development/fe_guide/vue.md | 82 | ||||
-rw-r--r-- | doc/development/fe_guide/vue3_migration.md | 2 |
15 files changed, 584 insertions, 86 deletions
diff --git a/doc/development/fe_guide/accessibility.md b/doc/development/fe_guide/accessibility.md index 5f22c13ca06..ab1325c67a9 100644 --- a/doc/development/fe_guide/accessibility.md +++ b/doc/development/fe_guide/accessibility.md @@ -15,39 +15,264 @@ This page contains guidelines we should follow. Since [no ARIA is better than bad ARIA](https://www.w3.org/TR/wai-aria-practices/#no_aria_better_bad_aria), review the following recommendations before using `aria-*`, `role`, and `tabindex`. -Use semantic HTML, which typically has accessibility semantics baked in, but always be sure to test with +Use semantic HTML, which has accessibility semantics baked in, and ideally test with [relevant combinations of screen readers and browsers](https://www.accessibility-developer-guide.com/knowledge/screen-readers/relevant-combinations/). In [WebAIM's accessibility analysis of the top million home pages](https://webaim.org/projects/million/#aria), they found that "ARIA correlated to higher detectable errors". It is likely that *misuse* of ARIA is a big cause of increased errors, -so when in doubt don't use `aria-*`, `role`, and `tabindex`, and stick with semantic HTML. - -## Provide accessible names to screen readers +so when in doubt don't use `aria-*`, `role`, and `tabindex` and stick with semantic HTML. + +## Quick checklist + +- [Text](#text-inputs-with-accessible-names), + [select](#select-inputs-with-accessible-names), + [checkbox](#checkbox-inputs-with-accessible-names), + [radio](#radio-inputs-with-accessible-names), + [file](#file-inputs-with-accessible-names), + and [toggle](#gltoggle-components-with-an-accessible-names) inputs have accessible names. +- [Buttons](#buttons-and-links-with-descriptive-accessible-names), + [links](#buttons-and-links-with-descriptive-accessible-names), + and [images](#images-with-accessible-names) have descriptive accessible names. +- Icons + - [Non-decorative icons](#icons-that-convey-information) have an `aria-label`. + - [Clickable icons](#icons-that-are-clickable) are buttons, that is, `<gl-button icon="close" />` is used and not `<gl-icon />`. + - Icon-only buttons have an `aria-label`. +- Interactive elements can be [accessed with the Tab key](#support-keyboard-only-use) and have a visible focus state. +- Are any `role`, `tabindex` or `aria-*` attributes unnecessary? +- Can any `div` or `span` elements be replaced with a more semantic [HTML element](https://developer.mozilla.org/en-US/docs/Web/HTML/Element) like `p`, `button`, or `time`? + +## Provide accessible names for screen readers To provide markup with accessible names, ensure every: - `input` has an associated `label`. -- `button` and `a` have child text, or `aria-label` when text isn’t present. - For example, an icon button with no visible text. +- `button` and `a` have child text, or `aria-label` when child text isn't present, such as for an icon button with no content. - `img` has an `alt` attribute. - `fieldset` has `legend` as its first child. - `figure` has `figcaption` as its first child. - `table` has `caption` as its first child. +Groups of checkboxes and radio inputs should be grouped together in a `fieldset` with a `legend`. +`legend` gives the group of checkboxes and radio inputs a label. + If the `label`, child text, or child element is not visually desired, use `.gl-sr-only` to hide the element from everything but screen readers. -Ensure the accessible name is descriptive enough to be understood in isolation. +### Examples of providing accessible names + +The following subsections contain examples of markup that render HTML elements with accessible names. + +Note that [when using `GlFormGroup`](https://bootstrap-vue.org/docs/components/form-group#accessibility): + +- Passing only a `label` prop renders a `fieldset` with a `legend` containing the `label` value. +- Passing both a `label` and a `label-for` prop renders a `label` that points to the form input with the same `label-for` ID. + +#### Text inputs with accessible names + +When using `GlFormGroup`, the `label` prop alone does not give the input an accessible name. +The `label-for` prop must also be provided to give the input an accessible name. + +Text input examples: + +```html +<!-- Input with label --> +<gl-form-group :label="__('Issue title')" label-for="issue-title"> + <gl-form-input id="issue-title" v-model="title" name="title" /> +</gl-form-group> + +<!-- Input with hidden label --> +<gl-form-group :label="__('Issue title')" label-for="issue-title" :label-sr-only="true"> + <gl-form-input id="issue-title" v-model="title" name="title" /> +</gl-form-group> +``` + +Textarea examples: + +```html +<!-- Textarea with label --> +<gl-form-group :label="__('Issue description')" label-for="issue-description"> + <gl-form-textarea id="issue-description" v-model="description" name="description" /> +</gl-form-group> + +<!-- Textarea with hidden label --> +<gl-form-group :label="__('Issue description')" label-for="issue-description" :label-sr-only="true"> + <gl-form-textarea id="issue-description" v-model="description" name="description" /> +</gl-form-group> +``` + +Alternatively, you can use a plain `label` element: + +```html +<!-- Input with label using `label` --> +<label for="issue-title">{{ __('Issue title') }}</label> +<gl-form-input id="issue-title" v-model="title" name="title" /> + +<!-- Input with hidden label using `label` --> +<label for="issue-title" class="gl-sr-only">{{ __('Issue title') }}</label> +<gl-form-input id="issue-title" v-model="title" name="title" /> +``` + +#### Select inputs with accessible names + +Select input examples: + +```html +<!-- Select input with label --> +<gl-form-group :label="__('Issue status')" label-for="issue-status"> + <gl-form-select id="issue-status" v-model="status" name="status" :options="options" /> +</gl-form-group> + +<!-- Select input with hidden label --> +<gl-form-group :label="__('Issue status')" label-for="issue-status" :label-sr-only="true"> + <gl-form-select id="issue-status" v-model="status" name="status" :options="options" /> +</gl-form-group> +``` + +#### Checkbox inputs with accessible names + +Single checkbox: + +```html +<!-- Single checkbox with label --> +<gl-form-checkbox v-model="status" name="status" value="task-complete"> + {{ __('Task complete') }} +</gl-form-checkbox> + +<!-- Single checkbox with hidden label --> +<gl-form-checkbox v-model="status" name="status" value="task-complete"> + <span class="gl-sr-only">{{ __('Task complete') }}</span> +</gl-form-checkbox> +``` + +Multiple checkboxes: + +```html +<!-- Multiple labeled checkboxes grouped within a fieldset --> +<gl-form-group :label="__('Task list')"> + <gl-form-checkbox name="task-list" value="task-1">{{ __('Task 1') }}</gl-form-checkbox> + <gl-form-checkbox name="task-list" value="task-2">{{ __('Task 2') }}</gl-form-checkbox> +</gl-form-group> + +<!-- Or --> +<gl-form-group :label="__('Task list')"> + <gl-form-checkbox-group v-model="selected" :options="options" name="task-list" /> +</gl-form-group> + +<!-- Multiple labeled checkboxes grouped within a fieldset with hidden legend --> +<gl-form-group :label="__('Task list')" :label-sr-only="true"> + <gl-form-checkbox name="task-list" value="task-1">{{ __('Task 1') }}</gl-form-checkbox> + <gl-form-checkbox name="task-list" value="task-2">{{ __('Task 2') }}</gl-form-checkbox> +</gl-form-group> + +<!-- Or --> +<gl-form-group :label="__('Task list')" :label-sr-only="true"> + <gl-form-checkbox-group v-model="selected" :options="options" name="task-list" /> +</gl-form-group> +``` + +#### Radio inputs with accessible names + +Single radio input: + +```html +<!-- Single radio with a label --> +<gl-form-radio v-model="status" name="status" value="opened"> + {{ __('Opened') }} +</gl-form-radio> + +<!-- Single radio with a hidden label --> +<gl-form-radio v-model="status" name="status" value="opened"> + <span class="gl-sr-only">{{ __('Opened') }}</span> +</gl-form-radio> +``` + +Multiple radio inputs: + +```html +<!-- Multiple labeled radio inputs grouped within a fieldset --> +<gl-form-group :label="__('Issue status')"> + <gl-form-radio name="status" value="opened">{{ __('Opened') }}</gl-form-radio> + <gl-form-radio name="status" value="closed">{{ __('Closed') }}</gl-form-radio> +</gl-form-group> + +<!-- Or --> +<gl-form-group :label="__('Issue status')"> + <gl-form-radio-group v-model="selected" :options="options" name="status" /> +</gl-form-group> + +<!-- Multiple labeled radio inputs grouped within a fieldset with hidden legend --> +<gl-form-group :label="__('Issue status')" :label-sr-only="true"> + <gl-form-radio name="status" value="opened">{{ __('Opened') }}</gl-form-radio> + <gl-form-radio name="status" value="closed">{{ __('Closed') }}</gl-form-radio> +</gl-form-group> + +<!-- Or --> +<gl-form-group :label="__('Issue status')" :label-sr-only="true"> + <gl-form-radio-group v-model="selected" :options="options" name="status" /> +</gl-form-group> +``` + +#### File inputs with accessible names + +File input examples: + +```html +<!-- File input with a label --> +<label for="attach-file">{{ __('Attach a file') }}</label> +<input id="attach-file" type="file" name="attach-file" /> + +<!-- File input with a hidden label --> +<label for="attach-file" class="gl-sr-only">{{ __('Attach a file') }}</label> +<input id="attach-file" type="file" name="attach-file" /> +``` + +#### GlToggle components with an accessible names + +`GlToggle` examples: ```html -// bad -<button>Submit</button> -<a href="url">page</a> +<!-- GlToggle with label --> +<gl-toggle v-model="notifications" :label="__('Notifications')" /> -// good -<button>Submit review</button> -<a href="url">GitLab's accessibility page</a> +<!-- GlToggle with hidden label --> +<gl-toggle v-model="notifications" :label="__('Notifications')" label-position="hidden" /> +``` + +#### GlFormCombobox components with an accessible names + +`GlFormCombobox` examples: + +```html +<!-- GlFormCombobox with label --> +<gl-form-combobox :label-text="__('Key')" :token-list="$options.tokenList" /> +``` + +#### Images with accessible names + +Image examples: + +```html +<img :src="imagePath" :alt="__('A description of the image')" /> + +<!-- SVGs implicitly have a graphics role so if it is semantically an image we should apply `role="img"` --> +<svg role="img" :alt="__('A description of the image')" /> +``` + +#### Buttons and links with descriptive accessible names + +Buttons and links should have accessible names that are descriptive enough to be understood in isolation. + +```html +<!-- bad --> +<gl-button @click="handleClick">{{ __('Submit') }}</gl-button> + +<gl-link :href="url">{{ __('page') }}</gl-link> + +<!-- good --> +<gl-button @click="handleClick">{{ __('Submit review') }}</gl-button> + +<gl-link :href="url">{{ __("GitLab's accessibility page") }}</gl-link> ``` ## Role @@ -81,31 +306,37 @@ element is interactive you must ensure: Use semantic HTML, such as `a` and `button`, which provides these behaviours by default. +Keep in mind that: + +- <kbd>Tab</kbd> and <kbd>Shift-Tab</kbd> should only move between interactive elements, not static content. +- When you add `:hover` styles, in most cases you should add `:focus` styles too so that the styling is applied for both mouse **and** keyboard users. +- If you remove an interactive element's `outline`, make sure you maintain visual focus state in another way such as with `box-shadow`. + See the [Pajamas Keyboard-only page](https://design.gitlab.com/accessibility-audits/2-keyboard-only/) for more detail. ## Tabindex Prefer **no** `tabindex` to using `tabindex`, since: -- Using semantic HTML such as `button` implicitly provides `tabindex="0"` -- Tabbing order should match the visual reading order and positive `tabindex`s interfere with this +- Using semantic HTML such as `button` implicitly provides `tabindex="0"`. +- Tabbing order should match the visual reading order and positive `tabindex`s interfere with this. ### Avoid using `tabindex="0"` to make an element interactive -Use interactive elements instead of `div`s and `span`s. +Use interactive elements instead of `div` and `span` tags. For example: -- If the element should be clickable, use a `button` -- If the element should be text editable, use an `input` or `textarea` +- If the element should be clickable, use a `button`. +- If the element should be text editable, use an `input` or `textarea`. Once the markup is semantically complete, use CSS to update it to its desired visual state. ```html -// bad +<!-- bad --> <div role="button" tabindex="0" @click="expand">Expand</div> -// good -<button @click="expand">Expand</button> +<!-- good --> +<gl-button @click="expand">Expand</gl-button> ``` ### Do not use `tabindex="0"` on interactive elements @@ -113,13 +344,13 @@ Once the markup is semantically complete, use CSS to update it to its desired vi Interactive elements are already tab accessible so adding `tabindex` is redundant. ```html -// bad -<a href="help" tabindex="0">Help</a> -<button tabindex="0">Submit</button> +<!-- bad --> +<gl-link href="help" tabindex="0">Help</gl-link> +<gl-button tabindex="0">Submit</gl-button> -// good -<a href="help">Help</a> -<button>Submit</button> +<!-- good --> +<gl-link href="help">Help</gl-link> +<gl-button>Submit</gl-button> ``` ### Do not use `tabindex="0"` on elements for screen readers to read @@ -129,10 +360,10 @@ The use of `tabindex="0"` is unnecessary and can cause problems, as screen reader users then expect to be able to interact with it. ```html -// bad -<span tabindex="0" :aria-label="message">{{ message }}</span> +<!-- bad --> +<p tabindex="0" :aria-label="message">{{ message }}</p> -// good +<!-- good --> <p>{{ message }}</p> ``` @@ -141,6 +372,57 @@ as screen reader users then expect to be able to interact with it. [Always avoid using `tabindex="1"`](https://webaim.org/techniques/keyboard/tabindex#overview) or greater. +## Icons + +Icons can be split into three different types: + +- Icons that are decorative +- Icons that convey meaning +- Icons that are clickable + +### Icons that are decorative + +Icons are decorative when there's no loss of information to the user when they are removed from the UI. + +As the majority of icons within GitLab are decorative, `GlIcon` automatically hides its rendered icons from screen readers. +Therefore, you do not need to add `aria-hidden="true"` to `GlIcon`, as this is redundant. + +```html +<!-- unnecessary — gl-icon hides icons from screen readers by default --> +<gl-icon name="rocket" aria-hidden="true" />` + +<!-- good --> +<gl-icon name="rocket" />` +``` + +### Icons that convey information + +Icons convey information if there is loss of information to the user when they are removed from the UI. + +An example is a confidential icon that conveys the issue is confidential, and does not have the text "Confidential" next to it. + +Icons that convey information must have an accessible name so that the information is conveyed to screen reader users too. + +```html +<!-- bad --> +<gl-icon name="eye-slash" />` + +<!-- good --> +<gl-icon name="eye-slash" :aria-label="__('Confidential issue')" />` +``` + +### Icons that are clickable + +Icons that are clickable are semantically buttons, so they should be rendered as buttons, with an accessible name. + +```html +<!-- bad --> +<gl-icon name="close" :aria-label="__('Close')" @click="handleClick" /> + +<!-- good --> +<gl-button icon="close" category="tertiary" :aria-label="__('Close')" @click="handleClick" /> +``` + ## Hiding elements Use the following table to hide elements from users, when appropriate. @@ -158,22 +440,24 @@ If the image is not an `img` element, such as an inline SVG, you can hide it by unnecessary when using `gl-icon`. ```html -// good - decorative images hidden from screen readers +<!-- good - decorative images hidden from screen readers --> + <img src="decorative.jpg" alt=""> -<svg role="img" alt=""> -<gl-icon name="epic"/> + +<svg role="img" alt="" /> + +<gl-icon name="epic" /> ``` -## When should ARIA be used +## When to use ARIA -No ARIA is required when using semantic HTML because it incorporates accessibility. +No ARIA is required when using semantic HTML, because it already incorporates accessibility. -However, there are some UI patterns and widgets that do not have semantic HTML equivalents. +However, there are some UI patterns that do not have semantic HTML equivalents. +General examples of these are dialogs (modals) and tabs. +GitLab-specific examples are assignee and label dropdowns. Building such widgets require ARIA to make them understandable to screen readers. -Proper research and testing should be done to ensure compliance with ARIA. - -Ideally, these widgets would exist only in [GitLab UI](https://gitlab-org.gitlab.io/gitlab-ui/). -Use of ARIA would then only occur in [GitLab UI](https://gitlab.com/gitlab-org/gitlab-ui/) and not [GitLab](https://gitlab.com/gitlab-org/gitlab/). +Proper research and testing should be done to ensure compliance with [WCAG](https://www.w3.org/WAI/standards-guidelines/wcag/). ## Resources diff --git a/doc/development/fe_guide/dependencies.md b/doc/development/fe_guide/dependencies.md index bf46e8e16ce..e8e251baafc 100644 --- a/doc/development/fe_guide/dependencies.md +++ b/doc/development/fe_guide/dependencies.md @@ -26,7 +26,7 @@ production assets post-compile. We use the [Renovate GitLab Bot](https://gitlab.com/gitlab-org/frontend/renovate-gitlab-bot) to automatically create merge requests for updating dependencies of several projects. -You can find the up-to-date list of projects managed by the renovate bot in the project’s README. +You can find the up-to-date list of projects managed by the renovate bot in the project's README. Some key dependencies updated using renovate are: diff --git a/doc/development/fe_guide/design_anti_patterns.md b/doc/development/fe_guide/design_anti_patterns.md index d230e413879..ee4fceff927 100644 --- a/doc/development/fe_guide/design_anti_patterns.md +++ b/doc/development/fe_guide/design_anti_patterns.md @@ -30,7 +30,7 @@ const createStore = () => new Vuex.Store({ // Notice that we are forcing all references to this module to use the same single instance of the store. // We are also creating the store at import-time and there is nothing which can automatically dispose of it. // -// As an alternative, we should simply export the `createStore` and let the client manage the +// As an alternative, we should export the `createStore` and let the client manage the // lifecycle and instance of the store. export default createStore(); ``` @@ -129,7 +129,7 @@ Here are some ills that Singletons often produce: This is because of the limitations of languages like Java where everything has to be wrapped in a class. In JavaScript we have things like object and function literals where we can solve -many problems with a module that simply exports utility functions. +many problems with a module that exports utility functions. ### When could the Singleton pattern be actually appropriate?** diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index 2e812d9fa0a..e87b4197269 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -43,13 +43,9 @@ can help you learn how to integrate Vue Apollo. For other use cases, check out the [Usage outside of Vue](#usage-outside-of-vue) section. -<!-- vale gitlab.Spelling = NO --> - -We use [Immer](https://immerjs.github.io/immer/docs/introduction) for immutable cache updates; +We use [Immer](https://immerjs.github.io/immer/) for immutable cache updates; see [Immutability and cache updates](#immutability-and-cache-updates) for more information. -<!-- vale gitlab.Spelling = YES --> - ### Tooling <!-- vale gitlab.Spelling = NO --> @@ -173,14 +169,10 @@ const primaryKeyId = getIdFromGraphQLId(data.id); From Apollo version 3.0.0 all the cache updates need to be immutable. It needs to be replaced entirely with a **new and updated** object. -<!-- vale gitlab.Spelling = NO --> - To facilitate the process of updating the cache and returning the new object we -use the library [Immer](https://immerjs.github.io/immer/docs/introduction). +use the library [Immer](https://immerjs.github.io/immer/). When possible, follow these conventions: -<!-- vale gitlab.Spelling = YES --> - - The updated cache is named `data`. - The original cache data is named `sourceData`. @@ -238,17 +230,33 @@ Read more about [Vue Apollo](https://github.com/vuejs/vue-apollo) in the [Vue Ap It is possible to manage an application state with Apollo by passing in a resolvers object when creating the default client. The default state can be set by writing -to the cache after setting up the default client. +to the cache after setting up the default client. In the example below, we are using query with `@client` Apollo directive to write the initial data to Apollo cache and then get this state in the Vue component: + +```javascript +// user.query.graphql + +query User { + user @client { + name + surname + age + } +} +``` ```javascript +// index.js + import Vue from 'vue'; import VueApollo from 'vue-apollo'; import createDefaultClient from '~/lib/graphql'; +import userQuery from '~/user/user.query.graphql' Vue.use(VueApollo); const defaultClient = createDefaultClient(); -defaultClient.cache.writeData({ +defaultClient.cache.writeQuery({ + query: userQuery, data: { user: { name: 'John', @@ -263,16 +271,15 @@ const apolloProvider = new VueApollo({ }); ``` -We can query local data with `@client` Apollo directive: - ```javascript -// user.query.graphql +// App.vue +import userQuery from '~/user/user.query.graphql' -query User { - user @client { - name - surname - age +export default { + apollo: { + user: { + query: userQuery + } } } ``` @@ -767,6 +774,66 @@ export default { }; ``` +#### Polling and Performance + +While the Apollo client has support for simple polling, for performance reasons, our [Etag-based caching](../polling.md) is preferred to hitting the database each time. + +Once the backend is set up, there are a few changes to make on the frontend. + +First, get your resource Etag path from the backend. In the example of the pipelines graph, this is called the `graphql_resource_etag`. This will be used to create new headers to add to the Apollo context: + +```javascript +/* pipelines/components/graph/utils.js */ + +/* eslint-disable @gitlab/require-i18n-strings */ +const getQueryHeaders = (etagResource) => { + return { + fetchOptions: { + method: 'GET', + }, + headers: { + /* This will depend on your feature */ + 'X-GITLAB-GRAPHQL-FEATURE-CORRELATION': 'verify/ci/pipeline-graph', + 'X-GITLAB-GRAPHQL-RESOURCE-ETAG': etagResource, + 'X-REQUESTED-WITH': 'XMLHttpRequest', + }, + }; +}; +/* eslint-enable @gitlab/require-i18n-strings */ + +/* component.vue */ + +apollo: { + pipeline: { + context() { + return getQueryHeaders(this.graphqlResourceEtag); + }, + query: getPipelineDetails, + pollInterval: 10000, + .. + }, +}, +``` + +Then, because Etags depend on the request being a `GET` instead of GraphQL's usual `POST`, but our default link library does not support `GET` we need to let our defaut Apollo client know to use a different library. + +```javascript +/* componentMountIndex.js */ + +const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient( + {}, + { + useGet: true, + }, + ), +}); +``` + +Keep in mind, this means your app will not batch queries. + +Once subscriptions are mature, this process can be replaced by using them and we can remove the separate link library and return to batching queries. + ### Testing #### Generating the GraphQL schema @@ -1377,6 +1444,34 @@ describe('My Index test with `createMockApollo`', () => { }); ``` +When you need to configure the mocked apollo client's caching behavior, +provide additional cache options when creating a mocked client instance and the provided options will merge with the default cache option: + +```javascript +const defaultCacheOptions = { + fragmentMatcher: { match: () => true }, + addTypename: false, +}; +``` + +```javascript +function createMockApolloProvider({ props = {}, requestHandlers } = {}) { + Vue.use(VueApollo); + + const mockApollo = createMockApollo( + requestHandlers, + {}, + { + dataIdFromObject: (object) => + // eslint-disable-next-line no-underscore-dangle + object.__typename === 'Requirement' ? object.iid : defaultDataIdFromObject(object), + }, + ); + + return mockApollo; +} +``` + ## Handling errors The GitLab GraphQL mutations have two distinct error modes: [Top-level](#top-level-errors) and [errors-as-data](#errors-as-data). diff --git a/doc/development/fe_guide/icons.md b/doc/development/fe_guide/icons.md index a7b62fbb267..ad344c00efd 100644 --- a/doc/development/fe_guide/icons.md +++ b/doc/development/fe_guide/icons.md @@ -104,7 +104,7 @@ by the examples that follow: **Example 1:** -The following HAML expression generates a loading icon’s markup and +The following HAML expression generates a loading icon's markup and centers the icon by wrapping it in a `gl-spinner-container` element. ```haml @@ -121,7 +121,7 @@ centers the icon by wrapping it in a `gl-spinner-container` element. **Example 2:** -The following HAML expression generates a loading icon’s markup +The following HAML expression generates a loading icon's markup with a custom size. It also appends a margin utility class. ```haml @@ -137,7 +137,7 @@ with a custom size. It also appends a margin utility class. ### Usage in Vue The [GitLab UI](https://gitlab-org.gitlab.io/gitlab-ui/) components library provides a -`GlLoadingIcon` component. See the component’s +`GlLoadingIcon` component. See the component's [storybook](https://gitlab-org.gitlab.io/gitlab-ui/?path=/story/base-loading-icon--default) for more information about its usage. diff --git a/doc/development/fe_guide/img/editor_lite_create_ext.png b/doc/development/fe_guide/img/editor_lite_create_ext.png Binary files differindex 73941cf5d62..9092c4b725c 100644 --- a/doc/development/fe_guide/img/editor_lite_create_ext.png +++ b/doc/development/fe_guide/img/editor_lite_create_ext.png diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 1315520342e..0f3754c29e7 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -11,7 +11,7 @@ across the GitLab frontend team. ## Overview -GitLab is built on top of [Ruby on Rails](https://rubyonrails.org). It uses [Haml](https://haml.info/) and a JavaScript0based frontend with [Vue.js](https://vuejs.org). +GitLab is built on top of [Ruby on Rails](https://rubyonrails.org). It uses [Haml](https://haml.info/) and a JavaScript-based frontend with [Vue.js](https://vuejs.org). <!-- vale gitlab.Spelling = NO --> Be wary of [the limitations that come with using Hamlit](https://github.com/k0kubun/hamlit/blob/master/REFERENCE.md#limitations). <!-- vale gitlab.Spelling = YES --> diff --git a/doc/development/fe_guide/performance.md b/doc/development/fe_guide/performance.md index 795de87d309..b6130335654 100644 --- a/doc/development/fe_guide/performance.md +++ b/doc/development/fe_guide/performance.md @@ -35,7 +35,7 @@ performance.getEntriesByName('my-component-start') - The start of navigation and a mark - The start of navigation and the moment the measurement is taken -It takes several arguments of which the measurement’s name is the only one required. Examples: +It takes several arguments of which the measurement's name is the only one required. Examples: - Duration between the start and end marks: @@ -185,16 +185,16 @@ To access stored measurements, you can use either: ### Naming convention All the marks and measures should be instantiated with the constants from -`app/assets/javascripts/performance/constants.js`. When you’re ready to add a new mark’s or -measurement’s label, you can follow the pattern. +`app/assets/javascripts/performance/constants.js`. When you're ready to add a new mark's or +measurement's label, you can follow the pattern. NOTE: This pattern is a recommendation and not a hard rule. ```javascript -app-*-start // for a start ‘mark’ -app-*-end // for an end ‘mark’ -app-* // for ‘measure’ +app-*-start // for a start 'mark' +app-*-end // for an end 'mark' +app-* // for 'measure' ``` For example, `'webide-init-editor-start`, `mr-diffs-mark-file-tree-end`, and so on. We do it to @@ -381,7 +381,7 @@ Use `webpackChunkName` when generating dynamic imports as it provides a deterministic filename for the chunk which can then be cached in the browser across GitLab versions. -More information is available in [webpack's code splitting documentation](https://webpack.js.org/guides/code-splitting/#dynamic-imports). +More information is available in [webpack's code splitting documentation](https://webpack.js.org/guides/code-splitting/#dynamic-imports) and [vue's dynamic component documentation](https://vuejs.org/v2/guide/components-dynamic-async.html). ### Minimizing page size diff --git a/doc/development/fe_guide/principles.md b/doc/development/fe_guide/principles.md index 4952d023d90..1bf37c8d008 100644 --- a/doc/development/fe_guide/principles.md +++ b/doc/development/fe_guide/principles.md @@ -18,4 +18,4 @@ There are multiple ways of writing code to accomplish the same results. We shoul ## Improve code [iteratively](https://about.gitlab.com/handbook/values/#iteration) -Whenever you see existing code that does not follow our current style guide, update it proactively. You don’t need to fix everything, but each merge request should iteratively improve our codebase, and reduce technical debt where possible. +Whenever you see existing code that does not follow our current style guide, update it proactively. You don't need to fix everything, but each merge request should iteratively improve our codebase, and reduce technical debt where possible. diff --git a/doc/development/fe_guide/security.md b/doc/development/fe_guide/security.md index 1a6646df877..79452327673 100644 --- a/doc/development/fe_guide/security.md +++ b/doc/development/fe_guide/security.md @@ -8,7 +8,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w ## Resources -[Mozilla’s HTTP Observatory CLI](https://github.com/mozilla/http-observatory-cli) and +[Mozilla's HTTP Observatory CLI](https://github.com/mozilla/http-observatory-cli) and [Qualys SSL Labs Server Test](https://www.ssllabs.com/ssltest/analyze.html) are good resources for finding potential problems and ensuring compliance with security best practices. @@ -41,7 +41,7 @@ Security Policy headers in the GitLab Rails app. Some resources on implementing Content Security Policy: - [MDN Article on CSP](https://developer.mozilla.org/en-US/docs/Web/Security/CSP) -- [GitHub’s CSP Journey on the GitHub Engineering Blog](http://githubengineering.com/githubs-csp-journey/) +- [GitHub's CSP Journey on the GitHub Engineering Blog](http://githubengineering.com/githubs-csp-journey/) - The Dropbox Engineering Blog's series on CSP: [1](https://blogs.dropbox.com/tech/2015/09/on-csp-reporting-and-filtering/), [2](https://blogs.dropbox.com/tech/2015/09/unsafe-inline-and-nonce-deployment/), [3](https://blogs.dropbox.com/tech/2015/09/csp-the-unexpected-eval/), [4](https://blogs.dropbox.com/tech/2015/09/csp-third-party-integrations-and-privilege-separation/) ### Subresource Integrity (SRI) diff --git a/doc/development/fe_guide/style/javascript.md b/doc/development/fe_guide/style/javascript.md index 334372af1f4..a2035eb1b55 100644 --- a/doc/development/fe_guide/style/javascript.md +++ b/doc/development/fe_guide/style/javascript.md @@ -98,16 +98,27 @@ class a { } ``` -## Use ParseInt +## Converting Strings to Integers -Use `ParseInt` when converting a numeric string into a number. +When converting strings to integers, `parseInt` has a slight performance advantage over `Number`, but `Number` is semantic and can be more readable. Prefer `parseInt`, but do not discourage `Number` if it significantly helps readability. + +**WARNING:** `parseInt` **must** include the [radix argument](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt). ```javascript // bad -Number('10') +parseInt('10'); + +// bad +things.map(parseInt) + +// ok +Number("106") + +// good +things.map(Number) // good -parseInt('10', 10); +parseInt("106", 10) ``` ## CSS Selectors - Use `js-` prefix @@ -297,7 +308,7 @@ Strive to write many small pure functions and minimize where mutations occur ## Export constants as primitives -Prefer exporting constant primitives with a common namespace over exporting objects. This allows for better compile-time reference checks and helps to avoid accidential `undefined`s at runtime. In addition, it helps in reducing bundle sizes. +Prefer exporting constant primitives with a common namespace over exporting objects. This allows for better compile-time reference checks and helps to avoid accidental `undefined`s at runtime. In addition, it helps in reducing bundle sizes. Only export the constants as a collection (array, or object) when there is a need to iterate over them, for instance, for a prop validator. diff --git a/doc/development/fe_guide/style/vue.md b/doc/development/fe_guide/style/vue.md index d62145b4a4c..93e4f234ccb 100644 --- a/doc/development/fe_guide/style/vue.md +++ b/doc/development/fe_guide/style/vue.md @@ -615,7 +615,7 @@ component state wherever possible. Instead, set the component's ``` 1. When asserting multiple props, check the deep equality of the `props()` object with -[`toEqual`](https://jestjs.io/docs/en/expect#toequalvalue): +[`toEqual`](https://jestjs.io/docs/expect#toequalvalue): ```javascript // good @@ -632,8 +632,8 @@ component state wherever possible. Instead, set the component's ``` 1. If you are only interested in some of the props, you can use -[`toMatchObject`](https://jestjs.io/docs/en/expect#tomatchobjectobject). Prefer `toMatchObject` -over [`expect.objectContaining`](https://jestjs.io/docs/en/expect#expectobjectcontainingobject): +[`toMatchObject`](https://jestjs.io/docs/expect#tomatchobjectobject). Prefer `toMatchObject` +over [`expect.objectContaining`](https://jestjs.io/docs/expect#expectobjectcontainingobject): ```javascript // good diff --git a/doc/development/fe_guide/troubleshooting.md b/doc/development/fe_guide/troubleshooting.md index 250fe5106d3..1b3991ee80d 100644 --- a/doc/development/fe_guide/troubleshooting.md +++ b/doc/development/fe_guide/troubleshooting.md @@ -66,3 +66,29 @@ TypeError: $ is not a function ``` **Remedy - Try moving the script into a separate repository and point to it to files in the GitLab repository** + +## Using Vue component issues + +### When rendering a component that uses GlFilteredSearch and the component or its parent uses Vue Apollo + +When trying to render our component GlFilteredSearch, you might get an error in the component's `provide` function: + +`cannot read suggestionsListClass of undefined` + +Currently, `vue-apollo` tries to [manually call a component's `provide()` in the `beforeCreate` part](https://github.com/vuejs/vue-apollo/blob/35e27ec398d844869e1bbbde73c6068b8aabe78a/packages/vue-apollo/src/mixin.js#L149) of the component lifecycle. This means that when a `provide()` references props, which aren't actually setup until after `created`, it will blow up. + +See this [closed MR](https://gitlab.com/gitlab-org/gitlab-ui/-/merge_requests/2019#note_514671251) for more context. + +**Remedy - try providing `apolloProvider` to the top-level Vue instance options** + +VueApollo will skip manually running `provide()` if it sees that an `apolloProvider` is provided in the `$options`. + +```patch + new Vue( + el, ++ apolloProvider: {}, + render(h) { + return h(App); + }, + ); +``` diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index 220a4a107aa..574faa0898f 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -99,6 +99,86 @@ return new Vue({ > When adding an `id` attribute to mount a Vue application, please make sure this `id` is unique across the codebase. +#### Providing Rails form fields to Vue applications + +When composing a form with Rails, the `name`, `id`, and `value` attributes of form inputs are generated +to match the backend. It can be helpful to have access to these generated attributes when converting +a Rails form to Vue, or when [integrating components (datepicker, project selector, etc)](https://gitlab.com/gitlab-org/gitlab/-/blob/8956ad767d522f37a96e03840595c767de030968/app/assets/javascripts/access_tokens/index.js#L15) into it. +The [`parseRailsFormFields`](https://gitlab.com/gitlab-org/gitlab/-/blob/fe88797f682c7ff0b13f2c2223a3ff45ada751c1/app/assets/javascripts/lib/utils/forms.js#L107) utility can be used to parse the generated form input attributes so they can be passed to the Vue application. +This allows us to easily integrate Vue components without changing how the form submits. + +```haml +-# form.html.haml += form_for user do |form| + .js-user-form + = form.text_field :name, class: 'form-control gl-form-input', data: { js_name: 'name' } + = form.text_field :email, class: 'form-control gl-form-input', data: { js_name: 'email' } +``` + +> The `js_name` data attribute is used as the key in the resulting JavaScript object. +For example `= form.text_field :email, data: { js_name: 'fooBarBaz' }` would be translated +to `{ fooBarBaz: { name: 'user[email]', id: 'user_email', value: '' } }` + +```javascript +// index.js +import Vue from 'vue'; +import { parseRailsFormFields } from '~/lib/utils/forms'; +import UserForm from './components/user_form.vue'; + +export const initUserForm = () => { + const el = document.querySelector('.js-user-form'); + + if (!el) { + return null; + } + + const fields = parseRailsFormFields(el); + + return new Vue({ + el, + render(h) { + return h(UserForm, { + props: { + fields, + }, + }); + }, + }); +}; +``` + +```vue +<script> +// user_form.vue +import { GlButton, GlFormGroup, GlFormInput } from '@gitlab/ui'; + +export default { + name: 'UserForm', + components: { GlButton, GlFormGroup, GlFormInput }, + props: { + fields: { + type: Object, + required: true, + }, + }, +}; +</script> + +<template> + <div> + <gl-form-group :label-for="fields.name.id" :label="__('Name')"> + <gl-form-input v-bind="fields.name" size="lg" /> + </gl-form-group> + + <gl-form-group :label-for="fields.email.id" :label="__('Email')"> + <gl-form-input v-bind="fields.email" type="email" size="lg" /> + </gl-form-group> + + <gl-button type="submit" category="primary" variant="confirm">{{ __('Update') }}</gl-button> + </div> +</template> +``` + #### Accessing the `gl` object We query the `gl` object for data that doesn't change during the application's life @@ -197,7 +277,7 @@ Check this [page](vuex.md) for more details. In the [Vue documentation](https://vuejs.org/v2/api/#Options-Data) the Data function/object is defined as follows: > The data object for the Vue instance. Vue recursively converts its properties into getter/setters -to make it “reactive”. The object must be plain: native objects such as browser API objects and +to make it "reactive". The object must be plain: native objects such as browser API objects and prototype properties are ignored. A rule of thumb is that data should just be data - it is not recommended to observe objects with their own stateful behavior. diff --git a/doc/development/fe_guide/vue3_migration.md b/doc/development/fe_guide/vue3_migration.md index ee25e97ab6e..d06e93da0f3 100644 --- a/doc/development/fe_guide/vue3_migration.md +++ b/doc/development/fe_guide/vue3_migration.md @@ -6,6 +6,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Migration to Vue 3 +Preparations for a Vue 3 migration are tracked in epic [&3174](https://gitlab.com/groups/gitlab-org/-/epics/3174) + In order to prepare for the eventual migration to Vue 3.x, we should be wary about adding the following features to the codebase: ## Vue filters |