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

Issue #3415660 by govind.maloo: Arrows supporting Previous and Next buttons on webforms are not rendering as expected #1182

Merged
merged 20 commits into from
Feb 15, 2024

Conversation

govindmaloo
Copy link
Collaborator

@govindmaloo govindmaloo commented Dec 18, 2023

https://www.drupal.org/project/civictheme/issues/3415660

Checklist before requesting a review

  • I have formatted the subject to include ticket number as [CS-123] Verb in past tense with dot at the end.
  • I have added a link to the issue tracker
  • I have provided information in Changed section about WHY something was done if this was not a normal implementation
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have run new and existing relevant tests locally with my changes, and they passed
  • I have provided screenshots, where applicable

Changed

Screenshots

@govindmaloo govindmaloo changed the base branch from develop to feature/CIVIC-1493 December 18, 2023 16:31
@govindmaloo
Copy link
Collaborator Author

@AlexSkrypnyk I have tried multiple approaches but only this is working for me. Let me know if you have any other suggestions.

@AlexSkrypnyk
Copy link
Contributor

@govindmaloo
Did you try to use the raw filter:

value: attributes.value|raw

// Fix htmlentity in submit button text.
$element = $variables['element'];

if (isset($element['#type']) && $element['#type'] == 'submit' && !empty($variables['attributes']['value'])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what if this is type reset or just a button? the solution should take care of all possible cases

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have tried raw filter but didn't work.

And it should work for all button. I can review if you think this solution can work.

@@ -17,7 +18,7 @@
{% set children %}
{% include "@atoms/button/button.twig" with {
type: type,
text: attributes.value,
Copy link
Contributor

Choose a reason for hiding this comment

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

does attributes.value contains HTML-converted text? can you try using raw filter?

@AlexSkrypnyk AlexSkrypnyk changed the title [CIVIC-1529] Fixed submit button text htmlentity issue. Issue #3415660 by govind.maloo: Arrows supporting Previous and Next buttons not rendering as expected Jan 23, 2024
@AlexSkrypnyk AlexSkrypnyk changed the title Issue #3415660 by govind.maloo: Arrows supporting Previous and Next buttons not rendering as expected Issue #3415660 by govind.maloo: Arrows supporting Previous and Next on webforms buttons not rendering as expected Jan 23, 2024
@AlexSkrypnyk AlexSkrypnyk changed the title Issue #3415660 by govind.maloo: Arrows supporting Previous and Next on webforms buttons not rendering as expected Issue #3415660 by govind.maloo: Arrows supporting Previous and Next buttons on webforms are not rendering as expected Jan 23, 2024
@AlexSkrypnyk
Copy link
Contributor

@govindmaloo
In order for this to pass QA, it needs to be tested on a real webform.

Could you please port the feedback webform from this PR #1163

Also, please port any required tests for that form.

Please do not include any CSS changes to the UI Kit - this is a purely preprocess issue and it should not require any style changes.

@AlexSkrypnyk AlexSkrypnyk added State: Requires more work Pull request was reviewed and reviver(s) asked to work further on the pull request and removed Question labels Jan 23, 2024
@govindmaloo
Copy link
Collaborator Author

@AlexSkrypnyk I have updated this branch on top of the latest develop and also added this PR to fix CI build
#1211

@@ -7,6 +7,7 @@
{% set classes = [
'container',
has_parent ? 'js-form-wrapper form-wrapper',
'ct-contianer',
Copy link
Contributor

Choose a reason for hiding this comment

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

there is no class ct-container. Also, this is misspelt

@AlexSkrypnyk AlexSkrypnyk changed the base branch from feature/CIVIC-1493 to develop February 7, 2024 00:50
@AlexSkrypnyk
Copy link
Contributor

@govindmaloo
I have updated the head branch to point to develop and this branch now contains unrelated changes.

Could you please remove any unrelated changes and only leave what is necessary for this issue.

Thanks

@govindmaloo
Copy link
Collaborator Author

@AlexSkrypnyk Please review I have updated the branch and removed webform related changes.

@govindmaloo govindmaloo added State: Needs review Pull requests needs a review from assigned developers and removed State: Requires more work Pull request was reviewed and reviver(s) asked to work further on the pull request labels Feb 8, 2024
@AlexSkrypnyk AlexSkrypnyk merged commit e54be55 into develop Feb 15, 2024
12 checks passed
@AlexSkrypnyk AlexSkrypnyk deleted the feature/CIVIC-1529 branch February 15, 2024 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
State: Needs review Pull requests needs a review from assigned developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants