Add a default placeholder title for newly added attributes and always show remove button for attributes #35904

nathanss · 2022-12-08T18:50:43Z

All Submissions:

Have you followed the WooCommerce Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Add a default placeholder title for newly added attributes and always show remove button for attributes

Screen.Recording.2022-12-09.at.10.14.18.mov

Closes #35116.

This PR is a very minor change/addition and does not require testing instructions (if checked you can ignore/remove the next section).

How to test the changes in this Pull Request:

Create a new product or edit an existing one.
Add at least two attributes.
Verify that the Remove button is always visible.
Verify that there's a placeholder title "Custom attribute"
Fill in the titles and verify that the placeholder gets updated
Test to see if no regressions occur in saving and re-ordering

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 created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?

FOR PR REVIEWER ONLY:

I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

… attribute

nathanss · 2022-12-08T18:51:26Z

@jarekmorawski I think we could improve the default placeholder title. How about "New custom attribute"?

github-actions · 2022-12-08T19:11:56Z

Test Results Summary

Commit SHA: 681e63c

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 53s
E2E Tests	187	0	0	6	0	193	19m 51s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

jarekmorawski · 2022-12-09T09:22:19Z

How about "New custom attribute"?

That'd make a good field value, but I think it'd be better to add a meaningful placeholder value that'd be shown in the field before the user starts typing. More like a tooltip than an actual field value entered by default. 🤔

e.g. Fabric or Brand

attributes-new.mov

nathanss · 2022-12-09T12:17:33Z

@jarekmorawski the placeholder definitely seems like a good idea and I can add it.

I already added the default title that shows up right after a new attribute is added. Do you think it's best to remove it and leave it blank, as it was before?

Screen.Recording.2022-12-09.at.09.10.03.mov

jarekmorawski · 2022-12-09T12:21:52Z

Oh, I like that! It's handy to have something in the title, too. Maybe we could change it to New custom attribute and make it grey or at least reduce its opacity to 40-50% until it's replaced with the actual attribute name?

nathanss · 2022-12-09T12:39:14Z

@jarekmorawski Let me investigate that. The PHP + jQuery architecture doesn't help sometimes, but it might be possible

…pacity class after user types the attribute name at the input

nathanss · 2022-12-09T13:17:10Z

@jarekmorawski

Screen.Recording.2022-12-09.at.10.14.18.mov

What do you think? I managed to do this. As I show in the video, there's just a small quirk in case the user fills in the attribute name, deletes it (blank) and the blur event occurs. In that case, the title is emptied. Do you think it's OK? I think it would rarely occur and the code to handle this would get too complex.

jarekmorawski · 2022-12-09T14:00:34Z

This looks perfectly fine! Thanks for exploring this, @nathanss 🙇 Now the only missing part is the updated Name field placeholder (e.g. Fabric or Brand instead of New attribute).

octaedro · 2022-12-13T18:58:25Z

plugins/woocommerce/includes/admin/meta-boxes/views/html-product-attribute.php

 							<input type="hidden" name="attribute_names[<?php echo esc_attr( $i ); ?>]" value="<?php echo esc_attr( $attribute->get_name() ); ?>" />
 						<?php else : ?>
-							<input type="text" class="attribute_name" name="attribute_names[<?php echo esc_attr( $i ); ?>]" value="<?php echo esc_attr( $attribute->get_name() ); ?>" />
+							<input type="text" class="attribute_name" name="attribute_names[<?php echo esc_attr( $i ); ?>]" value="<?php echo esc_attr( $attribute->get_name() ); ?>" placeholder="<?php esc_attr_e( 'New attribute', 'woocommerce' ); ?>" />


As far as I understood from this comment, it should say e.g. Fabric or Brand instead of New attribute.

And Custom attribute instead of e.g. Fabric or Brand here.

Great catch @octaedro, I added it to the wrong place. It would look pretty bad if we merged it like this:

LOL!

octaedro

Nice job @nathanss! This is testing well on my end and the code LGTM.
I only left a comment related to a placeholder.

I added e.g. Fabric or Brand to the wrong place.

octaedro

Thank you @nathanss! LGTM

… show remove button for attributes (#35904) * Remove CSS that hides the 'Remove' button for product attributes * Add default placeholder title 'Custom attribute' when user adds a new attribute * Add changelog * Add missing esc_html_e * Try to fix PHPCS * Add placeholder value for Attribute name input * Add css and logic to make placeholder title have opacity and remove opacity class after user types the attribute name at the input * Update placeholder value * Fix wrong labels I added e.g. Fabric or Brand to the wrong place.

Stas0238 · 2023-01-13T07:32:33Z

Hello everyone !
@nathanss @octaedro
Just updated to your latest WooCommerce version and checked this "new feature" for UI/UX but already found a minor issue .
If start typing then on mouseout of the input field the text above changes to the one I typed inside input and it is good but what will happen if I just decided to remove my text -> Placeholder text which was as first load seems to not showing but it would be good and needed. In my opinion there should be backup attribute with text value inside for those cases which I show on the video below.
Example: https://gyazo.com/a5648ee48261bd98e4069ce4cd8c6a7c
Hope you will include that fixpatch in next update !

nathanss added 3 commits December 8, 2022 15:42

Remove CSS that hides the 'Remove' button for product attributes

482d9fe

Add default placeholder title 'Custom attribute' when user adds a new…

e227aef

… attribute

Add changelog

2e666a9

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Dec 8, 2022

nathanss added 2 commits December 8, 2022 15:57

Add missing esc_html_e

661eb7f

Try to fix PHPCS

6020e42

nathanss requested a review from a team December 8, 2022 20:00

nathanss added 2 commits December 9, 2022 10:12

Add placeholder value for Attribute name input

5a791bd

Add css and logic to make placeholder title have opacity and remove o…

5a9f5d6

…pacity class after user types the attribute name at the input

Update placeholder value

5547b7e

octaedro requested review from octaedro and removed request for a team December 13, 2022 15:26

octaedro reviewed Dec 13, 2022

View reviewed changes

Fix wrong labels

681e63c

I added e.g. Fabric or Brand to the wrong place.

nathanss requested a review from octaedro December 13, 2022 19:20

octaedro approved these changes Dec 13, 2022

View reviewed changes

nathanss merged commit d19c204 into trunk Dec 14, 2022

nathanss deleted the enhancement/35116 branch December 14, 2022 12:50

github-actions bot added this to the 7.3.0 milestone Dec 14, 2022

github-actions bot mentioned this pull request Jan 13, 2023

Test WooCommerce 7.3 UVLabs/location-picker-at-checkout-for-woocommerce#59

Closed

nathanss mentioned this pull request Jan 16, 2023

[Enhancement]: Show placeholder in custom attribute title after deleting text from input #36448

Closed

Add a default placeholder title for newly added attributes and always show remove button for attributes #35904

Add a default placeholder title for newly added attributes and always show remove button for attributes #35904

Uh oh!

Conversation

nathanss commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

FOR PR REVIEWER ONLY:

Uh oh!

nathanss commented Dec 8, 2022

Uh oh!

github-actions bot commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results Summary

Uh oh!

jarekmorawski commented Dec 9, 2022

Uh oh!

nathanss commented Dec 9, 2022

Uh oh!

jarekmorawski commented Dec 9, 2022

Uh oh!

nathanss commented Dec 9, 2022

Uh oh!

nathanss commented Dec 9, 2022

Uh oh!

jarekmorawski commented Dec 9, 2022

Uh oh!

octaedro Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

octaedro Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

nathanss Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

octaedro left a comment

Choose a reason for hiding this comment

Uh oh!

octaedro left a comment

Choose a reason for hiding this comment

Uh oh!

Stas0238 commented Jan 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nathanss commented Dec 8, 2022 •

edited

Loading

github-actions bot commented Dec 8, 2022 •

edited

Loading