-
Notifications
You must be signed in to change notification settings - Fork 5
Form improvements for combination repository field #678
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
base: main
Are you sure you want to change the base?
Changes from all commits
f77510e
2f7c3d8
16f7e77
490aa0c
5b6add4
7cf171a
4f8277c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| {% load whitespaceless from ext_theme_tags %} | ||
|
|
||
| {% comment rst %} | ||
| Project repository combination field | ||
| ==================================== | ||
|
|
||
| This element wraps multiple fields and progressively enhances them to provide | ||
| two fields that interact with each other before form submission. It relies on | ||
| :js:class:`ProjectRepositoyMultifieldElement` for all of the added functionality. | ||
| {% endcomment %} | ||
|
|
||
| {# djlint wasn't happy with all of these attributes for some reason #} | ||
| {% whitespaceless as multifield_attrs %} | ||
| label="{{ multifield.label_html|safe }}" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where this label HTML comes from? I'm sure @stsewd will be 😟 when seeing a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm also not sure from where this comes from. But yeah, we should use mark_safe or format_html from the code instead of treating the labels as safe. I looked at the django code base, and labels are never marked as safe by default (only help texts, which is weird, but probably a legacy thing). |
||
| url-connected-services="{% url "socialaccount_connections" %}" | ||
| url-docs-manual="https://docs.readthedocs.com/platform/stable/guides/setup/git-repo-manual.html" | ||
| {{ multifield.flat_attrs|safe }} | ||
| {% endwhitespaceless %} | ||
|
|
||
| <readthedocs-project-repository-multifield {{ multifield_attrs|safe }}> | ||
| {{ fields_output|safe }} | ||
| </readthedocs-project-repository-multifield> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,218 @@ | ||
| import jquery from "jquery"; | ||
|
|
||
| import { html } from "lit"; | ||
| import { when } from "lit/directives/when.js"; | ||
| import { ref, createRef } from "lit/directives/ref.js"; | ||
|
|
||
| import { LightDOMElement } from "../application/elements"; | ||
|
|
||
| /** | ||
| * Field element web component base class | ||
| * | ||
| * This element wraps fields from Crispy/Django. These elements follow a progressive enhancement, rely on direct DOM manipulation instead of | ||
| * rendering the full element from a Lit template. This helps keeps parent web components purely rendered. | ||
| * | ||
| * These elements will be most helpful used from parent web components: | ||
| * | ||
| * .. code:: javascript | ||
| * | ||
| * render() { | ||
| * return html` | ||
| * <readthedocs-input-field | ||
| * .value="${this.value}" | ||
| * .disabled="${this.disabled}" | ||
| * @change="${this.handler}"> | ||
| * <input type="text" name="foo" /> | ||
| * </readthedocs-input-field> | ||
| * `; | ||
| * | ||
| * @property {String} value - Input field value for the form | ||
| * @property {Boolean} disabled - Is the field in a disabled state? Controls tab index | ||
| * @property {String} selector - CSS selector used to find the input in the light DOM | ||
| * @property {Boolean} hasError - Are there errors in the field | ||
| * @fires change - Event fired on attribute and error state changes | ||
| */ | ||
| export class FieldElement extends LightDOMElement { | ||
| static properties = { | ||
| value: { type: String }, | ||
| disabled: { type: Boolean }, | ||
| selector: { type: String }, | ||
| hasError: { type: Boolean }, | ||
| }; | ||
|
|
||
| /** @attr {Ref} refInput - Reference to the input element */ | ||
| refInput = createRef(); | ||
| /** @attr {Ref} refErrors - Reference to the errors list element */ | ||
| refErrors = createRef(); | ||
|
|
||
| constructor() { | ||
| super(); | ||
| this.selector = "input[name]"; | ||
| this.hasError = false; | ||
| } | ||
|
|
||
| /** | ||
| * Manually configure element references, we can't use a render template to | ||
| * establish element references via ``ref()``. | ||
| */ | ||
| connectedCallback() { | ||
| super.connectedCallback(); | ||
| this.refInput.value = this.querySelector(this.selector); | ||
| this.refErrors.value = this.querySelector(".ui.negative.label"); | ||
| this.hasError = Boolean(this.refErrors.value); | ||
| } | ||
|
|
||
| /** | ||
| * Set tab index on field to protect disabled fields from keyboard focus. | ||
| * | ||
| * Field types have differing elements that will need to alter tab index, so | ||
| * this can be overridden for any inherited classes. | ||
| */ | ||
| set tabIndex(tabIndex) { | ||
| const elemInput = this.refInput.value; | ||
| elemInput.setAttribute("tabIndex", tabIndex); | ||
| } | ||
|
|
||
| /** | ||
| * Setter for input field value, this can vary by subclass. | ||
| */ | ||
| set inputValue(value) { | ||
| const elemInput = this.refInput.value; | ||
| elemInput.value = this.value; | ||
| } | ||
|
|
||
| /** | ||
| * Lit updated properties from Lit property lifecycle. | ||
| * | ||
| * This is finally called for any properties , reflect these in the already | ||
| * rendered child elements. | ||
| * | ||
| * @param {Map} changed - properties that have changed on first update | ||
| */ | ||
| updated(changed) { | ||
| if (changed.has("value")) { | ||
| this.inputValue = this.value; | ||
| } | ||
| if (changed.has("disabled")) { | ||
| this.tabIndex = this.disabled ? -1 : 0; | ||
| } | ||
| if (changed.has("hasError") && this.hasError) { | ||
| this.dispatchChangeEvent(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Emit change event from this event | ||
| * | ||
| * In a Lit template, subscribe to this event in a render template with: | ||
| * ``<readthedocs-field @change="${this.someEventHandler}">``. | ||
| * | ||
| * @fires change - Change event fired on property changes and error state | ||
| */ | ||
| dispatchChangeEvent() { | ||
| this.dispatchEvent( | ||
| new CustomEvent("change", { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to trigger somebody else's |
||
| bubbles: false, | ||
| composed: true, | ||
| }), | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Clear error state and Crispy error elements | ||
| */ | ||
| clearErrors() { | ||
| if (this.hasError) { | ||
| const elemErrors = this.refErrors.value; | ||
| elemErrors.remove(); | ||
| this.hasError = false; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Field element for InputElement driven form fields | ||
| * | ||
| * Update events on the input element trigger changes here and emit events for | ||
| * changes in the parent element. | ||
| */ | ||
| class InputFieldElement extends FieldElement { | ||
| connectedCallback() { | ||
| super.connectedCallback(); | ||
| const elemInput = this.refInput.value; | ||
| if (elemInput) { | ||
| this.value = elemInput.value; | ||
| elemInput.addEventListener("change", (event) => { | ||
| this.dispatchChangeEvent(); | ||
| event.stopPropagation(); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| customElements.define("readthedocs-input-field", InputFieldElement); | ||
|
|
||
| /** | ||
| * Field element for SUI dropdown driven form fields | ||
| * | ||
| * This relies on the SUI dropdown module and module behaviors to drive all of | ||
| * the element properties and dropdown UI manipulation. | ||
| * | ||
| * @property {String} description - Rich select field choice description data | ||
| * @fires change - On dropdown select and initial element set up, fires event | ||
| */ | ||
| export class RichSelectFieldElement extends FieldElement { | ||
| static get properties() { | ||
| const properties = FieldElement.properties; | ||
| properties.description = { type: String }; | ||
| return properties; | ||
| } | ||
|
|
||
| constructor() { | ||
| super(); | ||
| this.selector = ".ui.dropdown"; | ||
| } | ||
|
|
||
| connectedCallback() { | ||
| super.connectedCallback(); | ||
| if (this.refInput.value) { | ||
| this.dropdown({ | ||
| fireOnInit: true, | ||
| onChange: (value, text, $selected) => { | ||
| // $selected is a jQuery element | ||
| const selected = $selected[0]; | ||
| this.value = selected.dataset.value; | ||
| this.description = selected.dataset.description; | ||
| this.dispatchChangeEvent(); | ||
| }, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Call SUI dropdown module | ||
| * | ||
| * This calls into the SUI jQuery ``dropdown`` module and can be used for | ||
| * both configuring the module or calling any of the behaviors the module | ||
| * provides (see https://fomantic-ui.com/modules/dropdown.html#behavior) | ||
| * | ||
| * @param {...*} args - All arguments to this functionm pass through | ||
| */ | ||
| dropdown(...args) { | ||
| return jquery(this.refInput.value).dropdown(...args); | ||
| } | ||
|
|
||
| set inputValue(value) { | ||
| // Call ``set selected`` behavior with ``value`` and ``preventChangeTrigger=true`` | ||
| // to avoid retriggering the ``onChange`` callback. | ||
| this.dropdown("set selected", value, true); | ||
| } | ||
|
|
||
| set tabIndex(tabIndex) { | ||
| // The module behavior ``set tabbable`` does not seem to work like we want, | ||
| // don't use it here and instead alter the input manually. | ||
| this.refInput.value | ||
| .querySelector("input.search") | ||
| .setAttribute("tabIndex", tabIndex); | ||
| } | ||
| } | ||
| customElements.define("readthedocs-richselect-field", RichSelectFieldElement); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| import * as avatar from "./avatar"; | ||
| import * as header from "./header"; | ||
| import * as filter from "./filter"; | ||
| import * as forms from "./forms"; | ||
| import * as menus from "./menus"; | ||
| import * as notifications from "./notifications"; | ||
| import * as popupcards from "./popupcards"; | ||
|
|
||
| export { avatar, header, filter, menus, notifications, popupcards }; | ||
| export { avatar, header, filter, forms, menus, notifications, popupcards }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the
as mutifield_attrshere for? I understand it gives it a name to the block, but shouldn't we use it when callingendwhitespacelesstoo? 🤔