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

Editor: Do not show 'Add animation' quick action on first page #11748

Merged
merged 4 commits into from
Jun 27, 2022

Conversation

mflorida
Copy link
Contributor

Context

Since animations cannot be applied to elements on the first page of a story, the 'Add animation' button should not be shown in the 'quick actions' button group.

Summary

The 'Add animation' button will not be rendered on the first page of a story.

Relevant Technical Choices

The currentPageNumber prop was tracked down and added to the dependency array for the function responsible for building the menu for 'common actions' of foreground elements. The logic for building the button group was changed to add buttons in the order they should appear

The logic for building the foregroundCommonActions button group was changed to conditionally render the add animation button based on the value of the currentPageNumber prop. The logic for rendering the 'reset' button was refactored using the same pattern for consistency.

To-do

None.

User-facing changes

Before:
Screen Shot 2022-06-20 at 12 57 59 PM

After:
Screen Shot 2022-06-20 at 12 56 44 PM

Testing Instructions

Navigate to the first page of a story. Click on an element (that would support animating on the other pages). The 'Add animation' button should not be rendered.

  • This is a non-user-facing change and requires no QA

Reviews

Does this PR have a security-related impact?

No

Does this PR change what data or activity we track or use?

No

Does this PR have a legal-related impact?

No

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Fixes #11602

@mflorida mflorida added Pod: Pea Group: Editing Main editing features Package: Story Editor /packages/story-editor labels Jun 20, 2022
@mflorida
Copy link
Contributor Author

Are the available properties in the state object of useStory listed anywhere? I had to console.log the object to find out there was a currentPageNumber property that could be referenced.

@mflorida
Copy link
Contributor Author

mflorida commented Jun 20, 2022

I'm unsure of other reviewers that should be added.

@swissspidy
Copy link
Collaborator

Are the available properties in the state object of useStory listed anywhere? I had to console.log the object to find out there was a currentPageNumber property that could be referenced.

The full list is here:

const fullStory = useMemo(
() => ({
pages,
currentPage,
currentPageId,
currentPageIndex,
currentPageNumber,
selectedElementIds,
selectedElements,
selectedElementAnimations,
hasSelection,
story,
animationState,
capabilities,
meta: {
isSaving: isSaving || isAutoSaving,
isSavingStory: isSaving,
isAutoSavingStory: isAutoSaving,
isFreshlyPublished,
isFreshlyPending,
},
copiedElementState,
}),
[
pages,
currentPage,
currentPageId,
currentPageIndex,
currentPageNumber,
selectedElementIds,
selectedElements,
selectedElementAnimations,
hasSelection,
story,
animationState,
capabilities,
isSaving,
isAutoSaving,
isFreshlyPublished,
isFreshlyPending,
copiedElementState,
]
);

@mflorida mflorida changed the title Editor: Don't render 'Add animation' button on first page of story Editor: Prevent rendering of 'Add animation' button on first page of story Jun 20, 2022
@sblinde
Copy link
Contributor

sblinde commented Jun 22, 2022

Took a look at this, and not sure if it's something with the QA instance (and haven't gotten to check this locally) but wasn't able to see the fix for this. When I added an image to the first page of a Story, I still saw the add animations quick button in the menu still.

@mflorida mflorida force-pushed the fix/11602-remove-first-page-add-animation-button branch from 1a9d33e to fc47b30 Compare June 22, 2022 19:42
@mflorida mflorida marked this pull request as ready for review June 22, 2022 19:42
@swissspidy
Copy link
Collaborator

Took a look at this, and not sure if it's something with the QA instance (and haven't gotten to check this locally) but wasn't able to see the fix for this. When I added an image to the first page of a Story, I still saw the add animations quick button in the menu still.

Changes are only available on the QA environment for non-draft PRs and after our bot has left a comment on the PR saying that ZIP files are available for testing.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2022

Size Change: +22 B (0%)

Total Size: 2.64 MB

ℹ️ View Unchanged
Filename Size Change
assets/css/carousel-view-rtl.css 702 B 0 B
assets/css/carousel-view.css 701 B 0 B
assets/css/web-stories-block-rtl.css 4.5 kB 0 B
assets/css/web-stories-block.css 4.55 kB 0 B
assets/css/web-stories-embed-rtl.css 318 B 0 B
assets/css/web-stories-embed.css 317 B 0 B
assets/css/web-stories-list-styles-rtl.css 2.34 kB 0 B
assets/css/web-stories-list-styles.css 2.37 kB 0 B
assets/css/web-stories-theme-style-twentyeleven-rtl.css 102 B 0 B
assets/css/web-stories-theme-style-twentyeleven.css 102 B 0 B
assets/css/web-stories-theme-style-twentyfifteen-rtl.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfifteen.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfourteen-rtl.css 287 B 0 B
assets/css/web-stories-theme-style-twentyfourteen.css 287 B 0 B
assets/css/web-stories-theme-style-twentyseventeen-rtl.css 288 B 0 B
assets/css/web-stories-theme-style-twentyseventeen.css 288 B 0 B
assets/css/web-stories-theme-style-twentysixteen-rtl.css 224 B 0 B
assets/css/web-stories-theme-style-twentysixteen.css 224 B 0 B
assets/css/web-stories-theme-style-twentyten-rtl.css 143 B 0 B
assets/css/web-stories-theme-style-twentyten.css 143 B 0 B
assets/css/web-stories-theme-style-twentytwelve-rtl.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwelve.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwenty-rtl.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwenty.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwentyone-rtl.css 326 B 0 B
assets/css/web-stories-theme-style-twentytwentyone.css 326 B 0 B
assets/css/web-stories-widget-rtl.css 482 B 0 B
assets/css/web-stories-widget.css 482 B 0 B
assets/css/wp-dashboard-rtl.css 657 B 0 B
assets/css/wp-dashboard.css 659 B 0 B
assets/css/wp-story-editor-rtl.css 737 B 0 B
assets/css/wp-story-editor.css 738 B 0 B
assets/js/1814.js 7.45 kB 0 B
assets/js/4422.js 49.3 kB 0 B
assets/js/5825.js 26.6 kB 0 B
assets/js/5980.js 5.48 kB 0 B
assets/js/7.js 1.14 MB 0 B
assets/js/7120.js 220 kB 0 B
assets/js/carousel-view.js 3.41 kB 0 B
assets/js/chunk-colorthief.js 2.64 kB 0 B
assets/js/chunk-ffmpeg.js 5.64 kB 0 B
assets/js/chunk-focus-visible.js 1.01 kB 0 B
assets/js/chunk-getStoryMarkup.js 5.83 kB 0 B
assets/js/chunk-html-to-image.js 4.6 kB 0 B
assets/js/chunk-opentype.js 96 B 0 B
assets/js/chunk-react-calendar.js 12.4 kB 0 B
assets/js/chunk-react-color.js 44.3 kB 0 B
assets/js/chunk-resize-observer-polyfill.js 2.57 kB 0 B
assets/js/chunk-web-animations-js.js 14.6 kB 0 B
assets/js/chunk-web-stories-template-0-metaData.js 546 B 0 B
assets/js/chunk-web-stories-template-0.js 10.6 kB 0 B
assets/js/chunk-web-stories-template-1-metaData.js 540 B 0 B
assets/js/chunk-web-stories-template-1.js 9.01 kB 0 B
assets/js/chunk-web-stories-template-10-metaData.js 533 B 0 B
assets/js/chunk-web-stories-template-10.js 6.91 kB 0 B
assets/js/chunk-web-stories-template-11-metaData.js 540 B 0 B
assets/js/chunk-web-stories-template-11.js 8.51 kB 0 B
assets/js/chunk-web-stories-template-12-metaData.js 496 B 0 B
assets/js/chunk-web-stories-template-12.js 9.48 kB 0 B
assets/js/chunk-web-stories-template-13-metaData.js 525 B 0 B
assets/js/chunk-web-stories-template-13.js 7.3 kB 0 B
assets/js/chunk-web-stories-template-14-metaData.js 582 B 0 B
assets/js/chunk-web-stories-template-14.js 7.58 kB 0 B
assets/js/chunk-web-stories-template-15-metaData.js 544 B 0 B
assets/js/chunk-web-stories-template-15.js 8.21 kB 0 B
assets/js/chunk-web-stories-template-16-metaData.js 588 B 0 B
assets/js/chunk-web-stories-template-16.js 10.3 kB 0 B
assets/js/chunk-web-stories-template-17-metaData.js 539 B 0 B
assets/js/chunk-web-stories-template-17.js 8.52 kB 0 B
assets/js/chunk-web-stories-template-18-metaData.js 585 B 0 B
assets/js/chunk-web-stories-template-18.js 9.05 kB 0 B
assets/js/chunk-web-stories-template-19-metaData.js 501 B 0 B
assets/js/chunk-web-stories-template-19.js 9.99 kB 0 B
assets/js/chunk-web-stories-template-2-metaData.js 586 B 0 B
assets/js/chunk-web-stories-template-2.js 9.16 kB 0 B
assets/js/chunk-web-stories-template-20-metaData.js 548 B 0 B
assets/js/chunk-web-stories-template-20.js 8.59 kB 0 B
assets/js/chunk-web-stories-template-21-metaData.js 534 B 0 B
assets/js/chunk-web-stories-template-21.js 9.16 kB 0 B
assets/js/chunk-web-stories-template-22-metaData.js 525 B 0 B
assets/js/chunk-web-stories-template-22.js 7.37 kB 0 B
assets/js/chunk-web-stories-template-23-metaData.js 605 B 0 B
assets/js/chunk-web-stories-template-23.js 6.99 kB 0 B
assets/js/chunk-web-stories-template-24-metaData.js 518 B 0 B
assets/js/chunk-web-stories-template-24.js 10.8 kB 0 B
assets/js/chunk-web-stories-template-25-metaData.js 544 B 0 B
assets/js/chunk-web-stories-template-25.js 7.07 kB 0 B
assets/js/chunk-web-stories-template-26-metaData.js 601 B 0 B
assets/js/chunk-web-stories-template-26.js 6.85 kB 0 B
assets/js/chunk-web-stories-template-27-metaData.js 543 B 0 B
assets/js/chunk-web-stories-template-27.js 7.36 kB 0 B
assets/js/chunk-web-stories-template-28-metaData.js 532 B 0 B
assets/js/chunk-web-stories-template-28.js 8.49 kB 0 B
assets/js/chunk-web-stories-template-29-metaData.js 561 B 0 B
assets/js/chunk-web-stories-template-29.js 8.49 kB 0 B
assets/js/chunk-web-stories-template-3-metaData.js 540 B 0 B
assets/js/chunk-web-stories-template-3.js 8.22 kB 0 B
assets/js/chunk-web-stories-template-30-metaData.js 576 B 0 B
assets/js/chunk-web-stories-template-30.js 7.67 kB 0 B
assets/js/chunk-web-stories-template-31-metaData.js 503 B 0 B
assets/js/chunk-web-stories-template-31.js 9.61 kB 0 B
assets/js/chunk-web-stories-template-32-metaData.js 551 B 0 B
assets/js/chunk-web-stories-template-32.js 12.2 kB 0 B
assets/js/chunk-web-stories-template-33-metaData.js 492 B 0 B
assets/js/chunk-web-stories-template-33.js 8.86 kB 0 B
assets/js/chunk-web-stories-template-34-metaData.js 571 B 0 B
assets/js/chunk-web-stories-template-34.js 7.57 kB 0 B
assets/js/chunk-web-stories-template-35-metaData.js 565 B 0 B
assets/js/chunk-web-stories-template-35.js 8.81 kB 0 B
assets/js/chunk-web-stories-template-36-metaData.js 576 B 0 B
assets/js/chunk-web-stories-template-36.js 11.6 kB 0 B
assets/js/chunk-web-stories-template-37-metaData.js 528 B 0 B
assets/js/chunk-web-stories-template-37.js 6.47 kB 0 B
assets/js/chunk-web-stories-template-38-metaData.js 572 B 0 B
assets/js/chunk-web-stories-template-38.js 7.96 kB 0 B
assets/js/chunk-web-stories-template-39-metaData.js 589 B 0 B
assets/js/chunk-web-stories-template-39.js 7.67 kB 0 B
assets/js/chunk-web-stories-template-4-metaData.js 565 B 0 B
assets/js/chunk-web-stories-template-4.js 11.5 kB 0 B
assets/js/chunk-web-stories-template-40-metaData.js 556 B 0 B
assets/js/chunk-web-stories-template-40.js 9.13 kB 0 B
assets/js/chunk-web-stories-template-41-metaData.js 572 B 0 B
assets/js/chunk-web-stories-template-41.js 7.75 kB 0 B
assets/js/chunk-web-stories-template-42-metaData.js 522 B 0 B
assets/js/chunk-web-stories-template-42.js 7 kB 0 B
assets/js/chunk-web-stories-template-43-metaData.js 558 B 0 B
assets/js/chunk-web-stories-template-43.js 8.37 kB 0 B
assets/js/chunk-web-stories-template-44-metaData.js 582 B 0 B
assets/js/chunk-web-stories-template-44.js 10.1 kB 0 B
assets/js/chunk-web-stories-template-45-metaData.js 564 B 0 B
assets/js/chunk-web-stories-template-45.js 7.12 kB 0 B
assets/js/chunk-web-stories-template-46-metaData.js 531 B 0 B
assets/js/chunk-web-stories-template-46.js 5.01 kB 0 B
assets/js/chunk-web-stories-template-47-metaData.js 592 B 0 B
assets/js/chunk-web-stories-template-47.js 8.46 kB 0 B
assets/js/chunk-web-stories-template-48-metaData.js 556 B 0 B
assets/js/chunk-web-stories-template-48.js 8.31 kB 0 B
assets/js/chunk-web-stories-template-49-metaData.js 518 B 0 B
assets/js/chunk-web-stories-template-49.js 9.7 kB 0 B
assets/js/chunk-web-stories-template-5-metaData.js 555 B 0 B
assets/js/chunk-web-stories-template-5.js 9.38 kB 0 B
assets/js/chunk-web-stories-template-50-metaData.js 504 B 0 B
assets/js/chunk-web-stories-template-50.js 8.26 kB 0 B
assets/js/chunk-web-stories-template-51-metaData.js 527 B 0 B
assets/js/chunk-web-stories-template-51.js 9.89 kB 0 B
assets/js/chunk-web-stories-template-52-metaData.js 602 B 0 B
assets/js/chunk-web-stories-template-52.js 10.1 kB 0 B
assets/js/chunk-web-stories-template-53-metaData.js 553 B 0 B
assets/js/chunk-web-stories-template-53.js 5.79 kB 0 B
assets/js/chunk-web-stories-template-54-metaData.js 547 B 0 B
assets/js/chunk-web-stories-template-54.js 7.52 kB 0 B
assets/js/chunk-web-stories-template-55-metaData.js 574 B 0 B
assets/js/chunk-web-stories-template-55.js 6.56 kB 0 B
assets/js/chunk-web-stories-template-56-metaData.js 543 B 0 B
assets/js/chunk-web-stories-template-56.js 9.5 kB 0 B
assets/js/chunk-web-stories-template-57-metaData.js 528 B 0 B
assets/js/chunk-web-stories-template-57.js 14.1 kB 0 B
assets/js/chunk-web-stories-template-58-metaData.js 556 B 0 B
assets/js/chunk-web-stories-template-58.js 5.61 kB 0 B
assets/js/chunk-web-stories-template-59-metaData.js 588 B 0 B
assets/js/chunk-web-stories-template-59.js 8.52 kB 0 B
assets/js/chunk-web-stories-template-6-metaData.js 569 B 0 B
assets/js/chunk-web-stories-template-6.js 7.04 kB 0 B
assets/js/chunk-web-stories-template-60-metaData.js 509 B 0 B
assets/js/chunk-web-stories-template-60.js 8.89 kB 0 B
assets/js/chunk-web-stories-template-7-metaData.js 569 B 0 B
assets/js/chunk-web-stories-template-7.js 7.21 kB 0 B
assets/js/chunk-web-stories-template-8-metaData.js 569 B 0 B
assets/js/chunk-web-stories-template-8.js 8.4 kB 0 B
assets/js/chunk-web-stories-template-9-metaData.js 581 B 0 B
assets/js/chunk-web-stories-template-9.js 8.49 kB 0 B
assets/js/chunk-web-stories-templates.js 443 B 0 B
assets/js/chunk-web-stories-textset-0.js 5.08 kB 0 B
assets/js/chunk-web-stories-textset-1.js 6.64 kB 0 B
assets/js/chunk-web-stories-textset-2.js 7.67 kB 0 B
assets/js/chunk-web-stories-textset-3.js 15.1 kB 0 B
assets/js/chunk-web-stories-textset-4.js 4.16 kB 0 B
assets/js/chunk-web-stories-textset-5.js 5.49 kB 0 B
assets/js/chunk-web-stories-textset-6.js 5.3 kB 0 B
assets/js/chunk-web-stories-textset-7.js 10.2 kB 0 B
assets/js/generateBlurhash.worker.worker.js 1.1 kB 0 B
assets/js/imgareaselect.js 3.77 kB 0 B
assets/js/lightbox.js 550 B 0 B
assets/js/tinymce-button.js 2.84 kB 0 B
assets/js/web-stories-activation-notice.js 26.9 kB 0 B
assets/js/web-stories-block.js 22.8 kB 0 B
assets/js/web-stories-embed.js 20 B 0 B
assets/js/web-stories-widget.js 587 B 0 B
assets/js/wp-dashboard.js 72.1 kB 0 B
assets/js/wp-story-editor.js 321 kB +22 B (0%)

compressed-size-action

@googleforcreators-bot
Copy link
Collaborator

googleforcreators-bot commented Jun 23, 2022

Plugin builds for a494252 are ready 🛎️!

@swissspidy
Copy link
Collaborator

^ Now it should work :)

@sblinde
Copy link
Contributor

sblinde commented Jun 23, 2022

Took a look at this, and not sure if it's something with the QA instance (and haven't gotten to check this locally) but wasn't able to see the fix for this. When I added an image to the first page of a Story, I still saw the add animations quick button in the menu still.

Changes are only available on the QA environment for non-draft PRs and after our bot has left a comment on the PR saying that ZIP files are available for testing.

🤦‍♀️ That makes total sense, did not realize this was in a draft status. Thank you!

Copy link
Contributor

@sblinde sblinde left a comment

Choose a reason for hiding this comment

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

🐝

Works great in test, approving.

In the future, if you're expecting review for a pull request, please open the PR regularly and not as a draft. Draft, ideally, should be used for things that need guidance / need to be readily shared with the larger group for conversation.

@mflorida mflorida requested a review from sblinde June 23, 2022 18:23
@sblinde sblinde added the Type: Enhancement New feature or improvement of an existing feature label Jun 24, 2022
@swissspidy swissspidy changed the title Editor: Prevent rendering of 'Add animation' button on first page of story Editor: Do not show 'Add animation' quick action on first page Jun 27, 2022
@swissspidy swissspidy merged commit dd2fa08 into main Jun 27, 2022
@swissspidy swissspidy deleted the fix/11602-remove-first-page-add-animation-button branch June 27, 2022 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Editing Main editing features Package: Story Editor /packages/story-editor Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor: 'Add animation' quick action button should not be shown on first page
4 participants