Skip to content

Commerce 15.3.0: Refund and partial refund #7037

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

Merged
merged 17 commits into from
Apr 29, 2025
Merged

Conversation

umbracotrd
Copy link
Contributor

Description

  • Add new pages explains Umbraco Commerce Refund and Partial Refund feature.
  • Update Commerce Payment Providers page about the new partial refund feature.

Type of suggestion

  • Typo/grammar fix
  • Updated outdated content
  • New content
  • Updates related to a new version
  • Other

Product & version (if relevant)

Umbraco Commerce 15.3.0

Deadline (if relevant)

When Umbraco Commerce 15.3.0 is released, which is in 3 or 4 weeks.

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@umbracotrd umbracotrd changed the title Commerce 15.3.0: Refund and partial refund WIP: Commerce 15.3.0: Refund and partial refund Apr 21, 2025
@umbracotrd
Copy link
Contributor Author

@mattbrailsford : this is the PR for Commerce refund feature. If you have any suggestions about new pages or content, please let me know.

@mattbrailsford
Copy link
Contributor

I think this is enough content wise, but I would suggest pulling out any text in the images and have it within the written text. Text in images is not accessible. By all means put boxes round things to highlight elements, but any descriptive text should be in the written docs, not embedded in the images.

@umbracotrd
Copy link
Contributor Author

Thanks @mattbrailsford , I've paraphrased the text in images and put them to the written docs. I still keep the text inside the images though.

@umbracotrd umbracotrd changed the title WIP: Commerce 15.3.0: Refund and partial refund Commerce 15.3.0: Refund and partial refund Apr 23, 2025
@sofietoft
Copy link
Contributor

Thanks for the PR @umbracotrd !

In order for us to review this properly, please add the new articles to the SUMMARY.md file.
Otherwise the articles will not show in the preview or on the published site once merged.

Also, please reconsider the text within the images.
As Matt mentions, this is not great for accessibility and if the images for some reason will not load, the user will miss out on important information, if the same is not added in the text.

@umbracotrd
Copy link
Contributor Author

umbracotrd commented Apr 24, 2025

@sofietoft : I've added new pages to the SUMMARY.md.

About the text within images, they're not exclusive information. All the information is copied as written text to the article, the texts inside images are just there to help those who can see. Removing texts from the images will just cripple majority of the audience.

Copy link
Contributor

@sofietoft sofietoft left a comment

Choose a reason for hiding this comment

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

Thanks again for the PR!

I've updated formatting and structure of the article to follow our styleguide.
Please remember to follow that, when making PR to the documentation 🙏

With the text from the images being added to the content as well, there is no reason to keep the text in the images.
As I see it, no information will be lost without it.
Keeping the text in the images will make the images harder to maintain should changes be made to the feature or UI in the future.
Adding an arrow to highlight a button or section is fine, as long as the color used matches out brand guidelines 🙌

Furthermore, the images should be added to an /images directory next to the articles where they are used. Learn more about the structure in the Styleguide.

I'd be happy to make the structural changes here and update the screenshots as well. However, that might slow down the timeline for when this PR will be merged.

@umbracotrd
Copy link
Contributor Author

umbracotrd commented Apr 25, 2025

About the text inside images, the argument about making the images harder to maintain can be said the same even if there're no text inside them. When we update a page's content, we need to update both written text and the images, it doesn't matter if the images contain texts or not. If we worry about the effort to maintain the images, how about we don't have them in the first place. No images = no maintenance.

Would you rather read the whole point 7. Fill in the fields in the Order Refund modal or see an image with red circles and arrows pointing exactly where you need to look and tell you which section is which on a single image? Like I said, removing the descriptive texts will just slow down the majority of the audience.

I'm not aware of our brand guidelines for arrow and square box. Please share the article if you have them. Or, I can replace all images by plain screenshots with nothing added to avoid the hassles.

@umbracotrd
Copy link
Contributor Author

@sofietoft : I've removed texts and drawings from images. They're just screenshots now.

Can docs team reconsider about the text inside images? Being inclusive and supporting accessibility shouldn't mean making it equally hard for everyone.

@umbracotrd umbracotrd requested a review from sofietoft April 26, 2025 02:25
Copy link
Contributor

@sofietoft sofietoft left a comment

Choose a reason for hiding this comment

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

Hi @umbracotrd !

Thanks for applying the suggestions and updating the images.
I see your point about it being quicker to understand and connect the information with the buttons/UI elements, when the text is in the images. But as mentioned, it is not good for accessibility and general maintenance of the docs.

I will discuss with the rest of team, whether we want to allow text within screenshots in the documentation going forward, or if there are alternative ways to go about adding information like this.

Thanks again for the PR.
I'll make sure it's merged when 15.3 is released 💪

@mattbrailsford
Copy link
Contributor

This is good to merge in now 👍

@sofietoft sofietoft merged commit bef1359 into main Apr 29, 2025
16 of 18 checks passed
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.

None yet

3 participants