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

Conversation

Sidsector9
Copy link
Member

@Sidsector9 Sidsector9 commented May 29, 2024

Description of the Change

  1. Added feature to refresh Content Resizing results without closing the results modal.
  2. 'Select' label is changed to 'Replace' within Content Resizing suggestions modal.
  3. Changed the Content Resizing modal title from static to dynamic depending on the type of resizing.

Closes #748

How to test the Change

Please follow the issue.

Changelog Entry

Added - Feature to refresh Content Resizing results without closing the results modal.
Changed - 'Select' label is changed to 'Replace' within Content Resizing suggestions modal.
Changed - Content Resizing modal title from static to dynamic depending on the type of resizing.

Credits

Props @jeffpaul @faisal-alvi @Sidsector9

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@Sidsector9 Sidsector9 requested review from dkotter, jeffpaul and a team as code owners May 29, 2024 15:30
@github-actions github-actions bot added this to the 3.1.0 milestone May 29, 2024
@github-actions github-actions bot added the needs:code-review This requires code review. label May 29, 2024
@Sidsector9 Sidsector9 marked this pull request as draft May 29, 2024 15:31
@github-actions github-actions bot removed the needs:code-review This requires code review. label May 29, 2024
@jeffpaul
Copy link
Member

@Sidsector9 I think @faisal-alvi's suggesting to use Replace instead of Insert might be better here... mind making that tweak?

@Sidsector9 Sidsector9 marked this pull request as ready for review May 29, 2024 16:57
@github-actions github-actions bot added the needs:code-review This requires code review. label May 29, 2024
@@ -330,6 +360,26 @@ const ContentResizingPlugin = () => {
</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

Comment on lines 151 to 153
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

@dkotter dkotter merged commit 12bc8e4 into develop May 30, 2024
13 of 14 checks passed
@dkotter dkotter deleted the enhance/748 branch May 30, 2024 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs:code-review This requires code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updates to expand/condense text functionality
3 participants