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

feat(correction): Add Priority setting & refactor Block #3844

Merged
merged 9 commits into from
Mar 24, 2025
Merged

feat(correction): Add Priority setting & refactor Block #3844

merged 9 commits into from
Mar 24, 2025

Conversation

Takshil-Kunadia
Copy link
Collaborator

@Takshil-Kunadia Takshil-Kunadia commented Mar 19, 2025 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

Completes the remainig changes of https://app.asana.com/0/1209292256643614/1209635079338293/f.

How to test the changes in this Pull Request:

  1. Create a post, add a few corrections with different locations.
  2. Add correction block and notice the filter by location setting in the sidebar.
  3. Select Top, Bottom & All option and see the corrections filtered by their location setting.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@Takshil-Kunadia Takshil-Kunadia marked this pull request as ready for review March 19, 2025 07:59
@Takshil-Kunadia Takshil-Kunadia requested a review from a team as a code owner March 19, 2025 07:59
@thomasguillot
Copy link
Contributor

In a block theme where elements can be freely moved, “Top” and “Bottom” Locations don’t seem meaningful.

If a block theme is active, renaming these locations to “High” and “Low” makes more sense, along with changing the terminology to Priority, (not just within the block but in the plugin too).

Having three separate blocks feels unnecessarily complex. Instead, let’s keep a single block with the Priority set to “All” by default.

@Takshil-Kunadia Takshil-Kunadia marked this pull request as draft March 21, 2025 07:11
@Takshil-Kunadia Takshil-Kunadia force-pushed the fix/corrections-blocks-location branch from 4fc587f to e9d7b38 Compare March 24, 2025 03:43
@Takshil-Kunadia Takshil-Kunadia changed the title feat(correction-blocks): Add location setting & block variations feat(correction): Add Priority setting & refactor Block Mar 24, 2025
@Takshil-Kunadia Takshil-Kunadia marked this pull request as ready for review March 24, 2025 03:46
@Takshil-Kunadia
Copy link
Collaborator Author

Hi @thomasguillot replaced "Location" setting with "Priority" in plugin & blocks.

cc: @leogermani

leogermani
leogermani approved these changes Mar 24, 2025
View reviewed changes
Copy link
Contributor

@leogermani leogermani left a comment

Choose a reason for hiding this comment

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

Looks good to me. I've refreshed with trunk and updated the README file

@github-actions github-actions bot added the [Status] Approved The pull request has been reviewed and is ready to merge label Mar 24, 2025
@leogermani leogermani merged commit 9232750 into trunk Mar 24, 2025
8 checks passed
@leogermani leogermani deleted the fix/corrections-blocks-location branch March 24, 2025 20:06
@github-actions GitHub Actions
Copy link

Hey @Takshil-Kunadia, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

matticbot pushed a commit that referenced this pull request Mar 26, 2025
@semantic-release-bot
chore(release): 6.2.0-alpha.4 [skip ci]
ac12855 
# [6.2.0-alpha.4](v6.2.0-alpha.3...v6.2.0-alpha.4) (2025-03-26)

### Bug Fixes

* add check if product before gating content ([#3850](#3850)) ([b9e385d](b9e385d))

### Features

* add canonical url to lite site single posts ([#3865](#3865)) ([471ad81](471ad81))
* add custom check for media visibility ([#3823](#3823)) ([c1d81dc](c1d81dc))
* **correction:** Add Priority setting & refactor Block ([#3844](#3844)) ([9232750](9232750))
* **corrections-modal:** fix modal action buttons & state handling ([#3852](#3852)) ([220d5d6](220d5d6))
* **corrections-modal:** Improve Corrections Modal UX ([#3835](#3835)) ([afc9844](afc9844))
* information architecture ([#3857](#3857)) ([6fb5951](6fb5951))
* update custom bylines data structure ([#3863](#3863)) ([9b66c1f](9b66c1f))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 6.2.0-alpha.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leogermani leogermani leogermani approved these changes

@thomasguillot thomasguillot Awaiting requested review from thomasguillot

Assignees
No one assigned
Labels
released on @alpha [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Takshil-Kunadia @thomasguillot @matticbot @leogermani