Skip to content

Refactor SiteContentForm and use not-allowed cursor for read-only string fields #2776

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.

Sign up for GitHub

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

Merged
pt2302 merged 6 commits into from
Nov 6, 2025
Merged

Refactor SiteContentForm and use not-allowed cursor for read-only string fields #2776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
992596a
Use Label widget for GDrive URL
pt2302 Oct 31, 2025
dfadd73
Refactor field rendering to make implementation cleaner
pt2302 Oct 31, 2025
e92290f
Reorder constants alphabetically
pt2302 Nov 3, 2025
a893d5e
Simplifying expression
pt2302 Nov 3, 2025
0010795
Revert Label changes for gdrive_url and modify string style instead
pt2302 Nov 5, 2025
3c60435
Move styling to forms.scss
pt2302 Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions gdrive_sync/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,26 @@
DRIVE_API_FILES = "files"
DRIVE_API_RESOURCES = [DRIVE_API_FILES, DRIVE_API_CHANGES]

DRIVE_FOLDER_VIDEOS_FINAL = "videos_final"
DRIVE_FOLDER_FILES_FINAL = "files_final"
DRIVE_FOLDER_FILES = "files"
DRIVE_FILE_FIELDS = "nextPageToken, files(id, name, md5Checksum, mimeType, createdTime, modifiedTime, size, webContentLink, trashed, parents)" # noqa: E501
DRIVE_MIMETYPE_FOLDER = "application/vnd.google-apps.folder"

DRIVE_FILE_CREATED_TIME = "createdTime"
DRIVE_FILE_DOWNLOAD_LINK = "webContentLink"
DRIVE_FILE_FIELDS = (
"nextPageToken, files(id, name, md5Checksum, mimeType, createdTime, "
"modifiedTime, size, webContentLink, trashed, parents)"
)
DRIVE_FILE_ID = "id"
DRIVE_FILE_NAME = "name"
DRIVE_FILE_MIME_TYPE = "mimeType"
DRIVE_FILE_MD5_CHECKSUM = "md5Checksum"
DRIVE_FILE_MIME_TYPE = "mimeType"
DRIVE_FILE_MODIFIED_TIME = "modifiedTime"
DRIVE_FILE_CREATED_TIME = "createdTime"
DRIVE_FILE_NAME = "name"
DRIVE_FILE_SIZE = "size"
DRIVE_FILE_DOWNLOAD_LINK = "webContentLink"
DRIVE_FILE_VIEW_URL = "https://drive.google.com/file/d/{file_id}/view"

DRIVE_FOLDER_FILES = "files"
DRIVE_FOLDER_FILES_FINAL = "files_final"
DRIVE_FOLDER_VIDEOS_FINAL = "videos_final"

DRIVE_MIMETYPE_FOLDER = "application/vnd.google-apps.folder"

VALID_TEXT_FILE_TYPES = [
".pdf",
".htm",
Expand Down
141 changes: 77 additions & 64 deletions static/js/components/forms/SiteContentForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,82 @@ export function FormFields(props: InnerFormProps): JSX.Element {
setDirty(dirty)
}, [setDirty, dirty])

const hideForNewVideoResource = (field: ConfigField) =>
configItem.name === "resource" &&
editorState.adding() &&
(field.name === "resourcetype" ||
field.name === "file" ||
field.name === "gdrive_url")

const showAsLabel = (field: ConfigField) =>
SETTINGS.gdrive_enabled &&
content?.type === "resource" &&
field.widget === WidgetVariant.File

const labelValue = (field: ConfigField) =>
filenameFromPath((values[field.name] as string) ?? "")

const renderField = (field: ConfigField) => {
if (hideForNewVideoResource(field)) return null

if (field.name === "title") {
return (
<React.Fragment key={field.name}>
<SiteContentField
field={field}
contentContext={contentContext}
onChange={handleChange}
/>
{content?.type === "page" ? (
<div>
<label htmlFor="page-url">Page URL</label>
<div className="help-text">
This is auto-generated from the title field
</div>
<Label value={`/pages/${content.filename}`} name="page-url" />
</div>
) : null}

@ChristopherChudzicki ChristopherChudzicki Nov 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize we showed this. It makes sense.

Thoughts:

  • the content url isn't really specific to just pages. It could be shown for other things.
  • It seems very central to WebsiteContent objects. We could make it mostly not special if we (A) add it as a (method) field on WebsiteContentDetailSerializer, (B) add it to the hugo config as a readOnly string field.
    • This makes it a lot less special and lets us control the help text via hugo config, and easily add different help text / title for pages, resources, etc.
    • it is still slightly special in that it would be a top-level WebsiteContent property, not nested under metadata, similar to "title" and "markdown". See contentInitialValues

If we did ☝️ , wouldn't we be able to remove all special handling (in this file) related to title? There's still some via contentInitialValues, since it's a top-level property.

</React.Fragment>
)
}

if (field.widget === WidgetVariant.Object) {
return (
<ObjectField
key={field.name}
field={field}
contentContext={contentContext}
values={values}
onChange={handleChange}
/>
)
}

if (showAsLabel(field)) {
return (
<div key={field.name}>
<label htmlFor={field.name}>{field.label}</label>
<Field
as={Label}
name={field.name}
value={labelValue(field)}
className="form-control"
onChange={handleChange}
/>
</div>
)
Comment on lines +199 to +210

@gemini-code-assist gemini-code-assist Bot Nov 3, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation for fields shown as labels results in an inconsistent DOM structure compared to other fields. Specifically, the <label> element ends up outside the form-group div, which is rendered by the Label component. This can lead to styling inconsistencies.

To ensure a consistent structure where the form-group div wraps both the label and the input, I suggest rendering the read-only input directly using Formik's Field component instead of using the Label component. This also makes the implementation self-contained and less dependent on the Label component's internal structure.

      return (
        <div key={field.name} className="form-group">
          <label htmlFor={field.name}>{field.label}</label>
          <Field
            as="input"
            type="text"
            id={field.name}
            name={field.name}
            value={labelValue(field)}
            className="form-control"
            readOnly
            style={{ cursor: "not-allowed" }}
          />
        </div>
      )

}

return (
<SiteContentField
key={field.name}
field={field}
contentContext={contentContext}
onChange={handleChange}
/>
)
}

return (
<Form
onSubmit={async (event) => {
Expand All @@ -158,70 +234,7 @@ export function FormFields(props: InnerFormProps): JSX.Element {
<div>
{renamedFields
.filter((field) => fieldIsVisible(field, values))
.map((field) => {
// Hide `resourcetype`, `file`, and `gdrive_url` in case of adding Video Resource (using YouTube ID)
if (
configItem.name === "resource" &&
(field.name === "resourcetype" ||
field.name === "file" ||
field.name === "gdrive_url") &&
editorState.adding()
) {
return null
}
if (field.name === "title") {
return (
<React.Fragment key={field.name}>
<SiteContentField
field={field}
contentContext={contentContext}
onChange={handleChange}
/>
{content?.type === "page" ? (
<div>
<label htmlFor="page-url">Page URL</label>
<div className="help-text">
This is auto-generated from the title field
</div>
<Label
value={`/pages/${content.filename}`}
name="page-url"
/>
</div>
) : null}
</React.Fragment>
)
}
return field.widget === WidgetVariant.Object ? (
<ObjectField
field={field}
key={field.name}
contentContext={contentContext}
values={values}
onChange={handleChange}
/>
) : SETTINGS.gdrive_enabled &&
content?.type === "resource" &&
field.widget === WidgetVariant.File ? (
<div key={field.name}>
<label htmlFor={field.name}>{field.label}</label>
<Field
as={Label}
name={field.name}
value={filenameFromPath(values[field.name] as string)}
className="form-control"
onChange={handleChange}
/>
</div>
) : (
<SiteContentField
field={field}
key={field.name}
contentContext={contentContext}
onChange={handleChange}
/>
)
})}
.map(renderField)}
</div>
<div className="form-group d-flex w-100 justify-content-end">
<button
Expand Down
7 changes: 7 additions & 0 deletions static/scss/forms.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ $form-border: 1px solid $studio-gray !important;
box-shadow: 0 1px $studio-gray;
}

.form-control[readonly],
.form-input[readonly],
input[readonly].form-control,
textarea[readonly].form-control {
cursor: not-allowed;
}

.ck.ck-editor__top {
border-bottom: $form-border;
}
Expand Down
Loading