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

fix:Order blocks in sidebar doesn't respect layout restrictions: requ… #6489

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alihamza1221
Copy link

@alihamza1221 alihamza1221 commented Nov 18, 2024


fixed : Order blocks in sidebar doesn't respect layout restrictions: required and fixed position

I added the checks to display the delete block and drag block button according to values set like fixed and required.
After my change:

order.block.fixes.video.mp4

Closes #6481

Copy link

netlify bot commented Nov 18, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit 51985c8
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/676e64153331be000851466c

@alihamza1221 alihamza1221 force-pushed the order-blocks-sidebar-fixs branch from 719a285 to 10fb29c Compare November 22, 2024 06:26
@alihamza1221 alihamza1221 marked this pull request as ready for review November 22, 2024 06:46
@alihamza1221
Copy link
Author

@plone/volto-team I have made the appropriate changes to fix the issue and It works as wanted please If you can review this PR.

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Please see my note about the change log entry.

Otherwise, this looks really good, but it should get a thorough technical review by the @plone/volto-team. Thank you!

packages/volto/news/6481.bugfix Outdated Show resolved Hide resolved
@stevepiercy stevepiercy requested a review from avoinea November 22, 2024 07:43
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

News looks good, still needs a technical review.

@fredvd
Copy link
Member

fredvd commented Dec 7, 2024

@alihamza1221 We had the yearly Plone Conference Last week in Brasilia, BR, so most of our community members who are volunteers in this project were very busy with preparing, travelling and attending. It could therefore take a while before this is reviewed, but it will probably get on the next Volto Team meeting.

As @sneridagh loves the new sidebar ordering he'll want improvements for that added asap ;-) (teasing him)

@@ -91,6 +99,9 @@ export const Item = forwardRef(
<button
ref={ref}
{...handleProps}
style={{
Copy link
Member

Choose a reason for hiding this comment

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

instead of having a visibility style it would make more sense to me to have the "disabled" property added and this way you also get a screen reader compatible information on why you can't interact with the button

Copy link
Author

Choose a reason for hiding this comment

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

Looks good, but there are other places were we have used visibilty style will update it today

Copy link
Author

Choose a reason for hiding this comment

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

@ichim-david I did added disabled property but removing visiblity style it's didn't seemed to respect the required functionality, If you would like to add more changes let me know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Order blocks in sidebar doesn't respect layout restrictions: required and fixed position
4 participants