Skip to content
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

Jump to bottom

QuickEdit: Add password field data to the pages quick edit #66567

Merged
merged 15 commits into from
Oct 31, 2024
Merged

QuickEdit: Add password field data to the pages quick edit #66567

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
fc76d1b
Add password field
louwie17 Sep 18, 2024
5dad068
Use password field in page data form
louwie17 Sep 18, 2024
83bf28a
Small change around hideLabelFromVision and styling fix
louwie17 Sep 19, 2024
e132f95
Stop relying on isValid for visibility
louwie17 Sep 19, 2024
a4c0d85
Disable sorting and hiding so it doesn't show in DataViews
louwie17 Sep 19, 2024
b21e81d
Add translation
louwie17 Oct 29, 2024
34dcc07
Remove old layout config for combined fields
louwie17 Oct 29, 2024
1a5147a
Move bulk edit filtering up to useMemo
louwie17 Oct 29, 2024
5d1234e
Update import
louwie17 Oct 29, 2024
c969697
Update class name
louwie17 Oct 29, 2024
67fa0b8
Update to use new isVisible function
louwie17 Oct 31, 2024
0f1b2dd
Remove duplicate label in password field and add type
louwie17 Oct 31, 2024
770c200
Make use of getValue instead of directly referencing the data object
louwie17 Oct 31, 2024
02b8eb0
Add border for password field in dataforms combined edit.
louwie17 Oct 31, 2024
6abc19e
Add JSDoc for password field export and update README
louwie17 Oct 31, 2024
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
4 changes: 4 additions & 0 deletions packages/dataviews/src/components/dataform-combined-edit/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,8 @@
&__field {
flex: 1 1 auto;
}

p.components-base-control__help:has(.components-checkbox-control__help) {
margin-top: $grid-unit-05;
}
}
68 changes: 38 additions & 30 deletions packages/edit-site/src/components/post-edit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ import { unlock } from '../../lock-unlock';

const { PostCardPanel } = unlock( editorPrivateApis );

const fieldsWithBulkEditSupport = [
'title',
'status',
'date',
'author',
'comment_status',
];

function PostEditForm( { postType, postId } ) {
const ids = useMemo( () => postId.split( ',' ), [ postId ] );
const { record } = useSelect(
Expand Down Expand Up @@ -58,27 +66,36 @@ function PostEditForm( { postType, postId } ) {
} ),
[ _fields ]
);
const form = {
type: 'panel',
fields: [
'featured_media',
'title',
'author',
'date',
'slug',
'parent',
'comment_status',
],
};

const fieldsWithBulkEditSupport = [
'title',
'status',
'date',
'author',
'comment_status',
];

const form = useMemo(
() => ( {
type: 'panel',
fields: [
'featured_media',
'title',
'status_and_visibility',
'author',
'date',
'slug',
'parent',
'comment_status',
].filter(
( field ) =>
ids.length === 1 ||
fieldsWithBulkEditSupport.includes( field )
),
combinedFields: [
{
id: 'status_and_visibility',
label: __( 'Status & Visibility' ),
children: [ 'status', 'password' ],
direction: 'vertical',
render: ( { item } ) => item.status,
},
],
} ),
[ ids ]
);
const onChange = ( edits ) => {
for ( const id of ids ) {
if (
Expand Down Expand Up @@ -117,16 +134,7 @@ function PostEditForm( { postType, postId } ) {
<DataForm
data={ ids.length === 1 ? record : multiEdits }
fields={ fields }
form={
ids.length === 1
? form
: {
...form,
fields: form.fields.filter( ( field ) =>
fieldsWithBulkEditSupport.includes( field )
),
}
}
form={ form }
onChange={ onChange }
/>
</VStack>
Expand Down
7 changes: 7 additions & 0 deletions packages/edit-site/src/components/post-edit/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,10 @@
justify-content: center;
}
}

.dataforms-layouts-panel__field-dropdown {
Copy link
Member

@oandregal oandregal Oct 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Shouldn't these styles be part of the dataviews/fields package instead? There's probably more places where we're doing this wrong, but DataViews specific classes shouldn't be used by consumers. They are not public API and can change without notice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not opposed to moving it to the password field specifically, there was two main reasons I end up doing it here:

  • Given the line is only relevant within the panel view in the status dropdown, it felled more like a higher level managed style.
  • I didn't want to make it the default for DataViews because of this previous conversation around combined fields

Having said that, I can add the same CSS to the password field specifically, so it is still panel specific.

Copy link
Member

Choose a reason for hiding this comment

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

mmm, I see your point. We need the combination of panel + field. These styles could live in edit-site (current), dataviews (but then would have wordpress field specific styles), or fields (but then would have dataviews-specific styles). None of the three options is ideal. Let's leave it as it is and keep an eye for similar things.

Copy link
Member

Choose a reason for hiding this comment

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

@louwie17 had a thought I wanted to share: what is this border for? is it for separating fields that are combined, like status & password are? If so, what if we refactor this to add the border automatically in that situation?

.fields-controls__password {
border-top: $border-width solid $gray-200;
padding-top: $grid-unit-20;
}
}
8 changes: 7 additions & 1 deletion packages/edit-site/src/components/post-fields/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import clsx from 'clsx';
*/
import { __, sprintf } from '@wordpress/i18n';
import { decodeEntities } from '@wordpress/html-entities';
import { featuredImageField, slugField, parentField } from '@wordpress/fields';
import {
featuredImageField,
slugField,
parentField,
passwordField,
} from '@wordpress/fields';
import {
createInterpolateElement,
useMemo,
Expand Down Expand Up @@ -348,6 +353,7 @@ function usePostFields( viewType ) {
},
],
},
passwordField,
],
[ authors, viewType, frontPageId, postsPageId ]
);
Expand Down
4 changes: 4 additions & 0 deletions packages/fields/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ Undocumented declaration.

This field is used to display the post parent.

### passwordField

Undocumented declaration.

### permanentlyDeletePost

Undocumented declaration.
Expand Down
1 change: 1 addition & 0 deletions packages/fields/src/fields/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export { default as titleField } from './title';
export { default as orderField } from './order';
export { default as featuredImageField } from './featured-image';
export { default as parentField } from './parent';
export { default as passwordField } from './password';
68 changes: 68 additions & 0 deletions packages/fields/src/fields/password/edit.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* WordPress dependencies
*/
import {
CheckboxControl,
__experimentalVStack as VStack,
TextControl,
} from '@wordpress/components';
import type { DataFormControlProps } from '@wordpress/dataviews';
import { useState } from '@wordpress/element';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import type { BasePost } from '../../types';

function PasswordEdit( {
data,
onChange,
field,
}: DataFormControlProps< BasePost > ) {
const [ showPassword, setShowPassword ] = useState(
!! field.getValue( { item: data } )
Copy link
Member

Choose a reason for hiding this comment

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

This is a nice detail, thanks.

);

const handleTogglePassword = ( value: boolean ) => {
setShowPassword( value );
if ( ! value ) {
onChange( { password: '' } );
}
};

return (
<VStack
as="fieldset"
spacing={ 4 }
className="fields-controls__password"
>
<CheckboxControl
__nextHasNoMarginBottom
label={ __( 'Password protected' ) }
help={ __( 'Only visible to those who know the password' ) }
checked={ showPassword }
onChange={ handleTogglePassword }
/>
{ showPassword && (
<div className="fields-controls__password-input">
<TextControl
label={ __( 'Password' ) }
onChange={ ( value ) =>
onChange( {
password: value,
} )
}
value={ field.getValue( { item: data } ) || '' }
placeholder={ __( 'Use a secure password' ) }
type="text"
__next40pxDefaultSize
__nextHasNoMarginBottom
maxLength={ 255 }
/>
</div>
) }
</VStack>
);
}
export default PasswordEdit;
22 changes: 22 additions & 0 deletions packages/fields/src/fields/password/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* WordPress dependencies
*/
import type { Field } from '@wordpress/dataviews';

/**
* Internal dependencies
*/
import type { BasePost } from '../../types';
import PasswordEdit from './edit';

const passwordField: Field< BasePost > = {
id: 'password',
Copy link
Member

Choose a reason for hiding this comment

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

What's this field type? I presume text. We should make the type mandatory at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes text would be the most appropriate. I added this as part of this commit: 0f1b2dd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although given this is a password, I wonder if we should add support for a new type to manage hidden texts? Something like hidden-text.
While the password is just plain text in the actual field, I wonder if we should display it as hidden when shown in a table? ( with maybe a tooltip or show button to display it ).
cc: @jameskoster if this could be of benefit

Copy link
Member

Choose a reason for hiding this comment

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

That's a sensible thought. I think text is good for now because the field is not visible in any place other than QuickEdit.

type: 'text',
getValue: ( { item } ) => item.password,
Edit: PasswordEdit,
enableSorting: false,
enableHiding: false,
isVisible: ( item ) => item.status !== 'private',
};

export default passwordField;
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a bit of JSDoc for this export, so it shows up in the docs (README) instead of "undocumented declaration"? (see parentField example)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup will do, one minute :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done: 6abc19e

Loading