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.

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

enhance/748: Updates to expand/condense text functionality #774

Merged
merged 9 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
3 changes: 2 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{

Check warning on line 1 in .eslintrc.json

View workflow job for this annotation

GitHub Actions / eslint

File ignored by default.
"globals": {
"wp": "readonly",
"jQuery": "readonly",
Expand All @@ -20,7 +20,8 @@
"File": "readonly",
"Headers": "readonly",
"requestAnimationFrame": "readonly",
"React": "readonly"
"React": "readonly",
"Block": "readonly"
},
"extends": ["plugin:@wordpress/eslint-plugin/recommended"],
"ignorePatterns": ["*.json"]
Expand Down
56 changes: 53 additions & 3 deletions src/js/gutenberg-plugins/content-resizing-plugin.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable @wordpress/no-unsafe-wp-apis */
/* eslint-disable no-shadow */
import { registerPlugin } from '@wordpress/plugins';
import {
store as blockEditorStore,
Expand All @@ -20,7 +21,7 @@
count as getWordCount,
count as getCharacterCount,
} from '@wordpress/wordcount';
import { __ } from '@wordpress/i18n';
import { __, sprintf } from '@wordpress/i18n';

import { DisableFeatureButton } from '../components';
import '../../scss/content-resizing-plugin.scss';
Expand Down Expand Up @@ -124,6 +125,9 @@
// Indicates if the modal window with the result is open/closed.
const [ isModalOpen, setIsModalOpen ] = useState( false );

// Modal title depending on resizing type.
const [ modalTitle, setModalTitle ] = useState( '' );

const { isMultiBlocksSelected, resizingType, isResizing } = useSelect(
( __select ) => {
return {
Expand All @@ -142,6 +146,12 @@
await resizeContent();
} )();
}

if ( 'grow' === resizingType ) {
setModalTitle( __( 'Expanded text suggestions', 'classifai' ) );
} else {
setModalTitle( __( 'Condensed text suggestions', 'classifai' ) );
Copy link
Collaborator

Choose a reason for hiding this comment

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

This may be more complicated than what we care about but in the case where someone has it set to only display a single suggestion, suggestions doesn't make sense at that point. Very minor thing so may be fine to just leave

Copy link
Member Author

Choose a reason for hiding this comment

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

Added here 6e9884d

}
}, [ resizingType ] );

// Triggers AJAX request to resize the content.
Expand All @@ -153,7 +163,7 @@
setIsModalOpen( true );
} )();
}
}, [ isResizing, selectedBlock ] );

Check warning on line 166 in src/js/gutenberg-plugins/content-resizing-plugin.js

View workflow job for this annotation

GitHub Actions / eslint

React Hook useEffect has a missing dependency: 'getResizedContent'. Either include it or remove the dependency array

// Resets to default states.
function resetStates() {
Expand All @@ -161,6 +171,23 @@
setTextArray( [] );
setIsModalOpen( false );
dispatch( resizeContentStore ).setResizingType( null );
setModalTitle( '' );
}

/**
* Refreshes results.
*
* @param {string} resizingType Type of resizing. grow|shrink|null
* @param {Block} selectedBlock The selected block.
*/
async function refreshResults( resizingType, selectedBlock ) {
dispatch( resizeContentStore ).setResizingType( null );

await new Promise( ( resolve ) => setTimeout( resolve, 0 ) );

setSelectedBlock( selectedBlock );

dispatch( resizeContentStore ).setResizingType( resizingType );
}

/**
Expand Down Expand Up @@ -261,7 +288,7 @@
// Result Modal JSX.
const suggestionModal = ! isResizing && textArray.length && isModalOpen && (
<Modal
title={ __( 'Select a suggestion', 'classifai' ) }
title={ modalTitle }
isFullScreen={ false }
className="classifai-content-resize__suggestion-modal"
onRequestClose={ () => {
Expand Down Expand Up @@ -316,7 +343,10 @@
</td>
<td>
<Button
text={ __( 'Select', 'classifai' ) }
text={ __(
'Replace',
'classifai'
) }
variant="secondary"
onClick={ () =>
updateContent( textItem )
Expand All @@ -330,6 +360,26 @@
</tbody>
</table>
</div>
<p>
{ __(
'None of these suggestions are ideal, please',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with your thought here: #748 (comment). To me this reads as if something went wrong and we're giving an error message. It also doesn't make sense if someone has it set to only show a single result (suggestions vs suggestion).

I also wonder if a button makes more sense here, as that would stand out more, especially when the disable functionality is turned on, we already show a link here:

Content resizing modal

Something like this:

Refresh results in modal

I also think Retry expanding/condensing the text looks good here, so I'm fine with either text:

Retry button in modal

'classifai'
) }{ ' ' }
<Button
onClick={ () =>
refreshResults( resizingType, selectedBlock )
}
variant="link"
>
{ sprintf(
/* translators: %s type of resizing */
__( 'retry %s', 'classifai' ),
'grow' === resizingType
? __( 'expanding the text', 'classifai' )
: __( 'condensing the text', 'classifai' )
) }
</Button>
</p>
<DisableFeatureButton feature="feature_content_resizing" />
</Modal>
);
Expand Down
Loading