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

MWPW-162614: Normalize UX for field updates #100

Merged
merged 28 commits into from
Dec 11, 2024
Merged

MWPW-162614: Normalize UX for field updates #100

merged 28 commits into from
Dec 11, 2024

Conversation

yesil
Copy link
Collaborator

@yesil yesil commented Nov 20, 2024

Refactored the editor panel in own custom element
added unit tests.
Copy link

aem-code-sync bot commented Nov 20, 2024

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

Copy link

aem-code-sync bot commented Nov 20, 2024

Page Scores Audits Google
📱 /studio.html PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /studio.html PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@@ -91,7 +91,7 @@ describe('Multifield', () => {
).to.equal(1);
});

it('should support mas-mnemonic-field', async () => {
it.only('should support mas-mnemonic-field', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

.only

@afmicka
Copy link
Collaborator

afmicka commented Nov 28, 2024

@yesil when you make a change to any of the fields, it does get rendered immediately in the preview but the status dot stays green (even if you move focus from the field), does not indicate any of the changes (should turn into yellow after any change and blue when saved but not yet published) .
Screenshot 2024-11-28 at 12 50 58

@afmicka
Copy link
Collaborator

afmicka commented Nov 28, 2024

@yesil Also, when you add a new link to the description RTE and make a change to the text, changes are not rendered in the field (not before and not after the form save). But the changes are rendered in the card itself. This is NOT a regression but maybe is related to the other fixes so can be added
Screenshot 2024-11-28 at 13 58 17

Screenshot 2024-11-28 at 14 01 06

@yesil
Copy link
Collaborator Author

yesil commented Nov 28, 2024

@afmicka the link update issue is similar to MWPW-162740.
I pulled the fix I proposed to @Axelcureno at https://github.com/adobecom/mas/compare/MWPW-162740...yesil:mas:MWPW-162740?expand=1.

Please try again let me know.

@npeltier npeltier mentioned this pull request Nov 28, 2024
@afmicka
Copy link
Collaborator

afmicka commented Nov 28, 2024

Those changes are not validated. It made it worse now for the link editing. It does not render at all the changes in the RTE. Earlier at least it worked if the link already exist in the field
Screenshot 2024-11-28 at 15 29 56

@afmicka
Copy link
Collaborator

afmicka commented Dec 10, 2024

Looks good to me

@afmicka afmicka added verified and removed verified labels Dec 10, 2024
@afmicka
Copy link
Collaborator

afmicka commented Dec 11, 2024

@yesil I have removed the verified label. I checked the studio with your CSS switch branch and it is breaking the view. On hover cards are pitch black. I know you have MAS PR to support CSS change too that might handle this but then i am not sure how to proceed with merges in MAS

https://mwpw-162614--mas--adobecom.hlx.page/studio.html?milolibs=MWPW-163479--milo--adobecom#path=nala
Screenshot 2024-12-11 at 10 41 28

@yesil yesil merged commit 75ccf9a into main Dec 11, 2024
4 of 5 checks passed
@yesil yesil deleted the MWPW-162614 branch December 11, 2024 17:07
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.

4 participants