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

kie-issues#944: On the DMN Editor, all buttons that have no displayed text should have a title. #2163

Merged
merged 3 commits into from
Feb 21, 2024

Conversation

tiagobento
Copy link
Contributor

@@ -197,6 +199,7 @@ export function DataTypes() {
id="add-data-type-toggle"
splitButtonItems={[
<DropdownToggleAction
{...extraPropsForDropdownToggleAction}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not inline it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment...

Copy link
Contributor

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

@tiagobento thank you for adding this improvement. It will be beneficial as for human users so for playwright testing.

I have one question about general titles we add:

  • "Add"
  • "Remove"
  • "Close"

Would it be possible to add more detail into similar titles?

  • "Add [something]"
  • "Remove [something]"
  • "Close [something]"

I understand it is probably not needed for human users, who understand from context, what will be "Added", "Removed" or "Closed", however if we do "page.getByTitle('Add')", it can match multiple elements and it may be difficult to write short tests then.

What is your opinion please?

@tiagobento
Copy link
Contributor Author

@jomarko Yeah, I preferred to leave it as inferable from context, but I see your point. I can change it to be more specific, of course. However, I must say that we shouldn't start compromising on UI/UX in order for tests to be more easily written. I think we should of course keep tests in mind with everything that we do. IMHO we should always think what's the best UX possible for users, then figure out how to test it.

Copy link
Contributor

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

@tiagobento thank you.

@tiagobento tiagobento merged commit 47d9eb9 into apache:main Feb 21, 2024
8 checks passed
paulovmr pushed a commit to kiegroup/kie-tools that referenced this pull request Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

On the DMN Editor, all buttons that have no displayed text should have a title.
3 participants