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

Allow alt text annotations to be deleted #45

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

gert-janvercauteren
Copy link
Contributor

@gert-janvercauteren gert-janvercauteren commented Dec 17, 2024

Description

  • Add a delete button for alt text annotations, similar to heading annotations
  • When we can now add image annotations manually, we can't undo a 'mistake' once added, this PR resolves that.
  • Aligned the n/a and input field so the text is visually aligned
  • Added 'grow' to middle element, so the select and delete button are always aligned at the end of the row
  • Locked the 'alt' text layer, so it can always be visible and not interfere with selection of the layers below

image

Checklist:

  • I have read the CONTRIBUTING document and agree to the project's Code of Conduct
  • I have updated/added documentation affected by my changes (in DOCS.md).

@calebnance calebnance self-requested a review December 17, 2024 17:56
@calebnance calebnance added the enhancement New feature or request label Dec 17, 2024
@@ -28,7 +28,7 @@
"label": "Alternative text",
"designerCheck": "Alternative text",
"path": "alt-text",
"shouldHide": true,
"shouldHide": false,
"percent": 20
Copy link
Collaborator

Choose a reason for hiding this comment

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

@gert-janvercauteren what was the reason for this change? it's breaking the native flow for me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not on purpose!

@@ -292,6 +293,7 @@ export const add = (msg) => {
// update with id (for future scanning)
altTextFrame.name = `${altTextLayerName} | ${altTextFrame.id}`;
altTextFrame.expanded = false;
altTextFrame.locked = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

src/ui.js Outdated

// should hide flag is true and step data exists
if (shouldHide === true && stepsDataKeysArray.includes(key)) {
// if not current path, hide layer + force hide override
if (currentPath !== path || forceHide === true) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@gert-janvercauteren all of this still needs to be there, this hides the annotation layer so that when users add images manually, they don't accidentally select the annotation layer(s) when trying to select layers from the design file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the locked alt text layer, you won't be able to select it.
I'll revert, as this is not the main aim for the changes :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants