-
Notifications
You must be signed in to change notification settings - Fork 203
Fix: Receipt header heading and description nested elements. #8216
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
Open
gilbert-hernandez
wants to merge
3
commits into
develop
Choose a base branch
from
fix/GIVE-3061
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+5
−4
Open
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
dd8bbc6
GIVE-3061 Strip nested p tags from receipt header heading and descrip…
gilbert-hernandez 4981324
KAD-3061 Requested changes. Remove hardcoded elements and update style.
gilbert-hernandez 08dd6d9
GIVE-3061 Add unreleased tag to docblock
gilbert-hernandez File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gilbert-hernandez i'd like to suggest a different approach. Although this will sort of fix the issue, it will hinder the intended flexibility of these fields. The idea here is customers can use the rich text field to edit the heading and description fields for the donation confirmation - but as you can see we hardcoded the tagName which kind of defeats the purpose of using a rich text 😏
People may want to add additional information to the donation confirmation that should actually render whatever tags they want to use (including p tags)
So what I would suggest, is we make these both wrapped in divs and let the rich text editor determine their tag. Then, we can update the initial styling here to target any tag. (people can always use custom css later to get specific).
I also don't like the fact we are hardcoding an h1 anyway.
We did something similar with the styling of the Form Header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonwaldstein
That makes sense. I see now that the heading and description are wysiwyg components and the inline styles there will overrule the default style, which are applied to any child element with this update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gilbert-hernandez yup! thanks for the update.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gilbert-hernandez oh one more thing - we typically add
@unreleasedabove an existing@sincetag to mark the change. You should be able to add one to this file