SHS-5683: Blockquote implementation #1575
Merged
Conversation
This file contains 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
… shs-5683-blockquote-implementation
cienvaras
requested changes
Jul 12, 2024
There was a problem hiding this comment.
@mariannuar Looks great overall. Just some minor fixes needed, let me know if you have any questions. Please notice I updated the branch with the latest code in fk-stndfd-sprint-54.
docroot/themes/humsci/humsci_basic/src/scss/ckeditor/imports.scss
Outdated
Show resolved
Hide resolved
docroot/themes/humsci/humsci_basic/src/scss/tools/_mixins.text.scss
Outdated
Show resolved
Hide resolved
docroot/themes/humsci/humsci_basic/src/scss/tools/_mixins.text.scss
Outdated
Show resolved
Hide resolved
docroot/themes/humsci/humsci_basic/src/scss/tools/_mixins.text.scss
Outdated
Show resolved
Hide resolved
docroot/themes/humsci/humsci_basic/src/scss/ckeditor/imports.scss
Outdated
Show resolved
Hide resolved
added 2 commits
July 11, 2024 22:46
…DO/suhumsci into shs-5683-blockquote-implementation
|
@cienvaras Thank you! This is ready for review again 🙂 |
cienvaras
approved these changes
Jul 12, 2024
There was a problem hiding this comment.
@mariannuar Thanks for the changes, everything look better now, specially the quote icons.
I added some extra changes:
- Moved some of the overrides to the testimonial component styles, to prevent doing extra overrides in the mixin. This reduces the code size a bit and prevents overriding some basic styles for all the blockquotes.
- Removed the
&:not(.hb-testimonial__quote)styles fromimport.scssand added back the!important. While your solution works, it adds an extra class to the CKEditor styles that's not relevant here. If the class changes in the future or if we need to override a new class, then we would need to do extra changes here. We need to ensure that all blockquotes in CKEditor have the same styles, and!importantis the best approach for that in this case. At least you were able to remove the font related styles because they were not needed anymore.
Let me know if you have any question about my changes!
cienvaras
added
Passes code review
Passes functional review
and removed
Needs review
labels
joegl
pushed a commit
that referenced
this pull request
Jul 15, 2024
* feat(shs-5663): make editoria11y work as expected (#1570) * feat(shs-5671): add bigger preview images in the editing experience (#1573) * Remove legacy themes modules (#1562) * feat(shs-5631): uninstall legacy themes and remove related configuration * chore(shs-5631): update documentation to remove references to legacy themes * feat: remove legacy themes files * feat: remove hs_mathematics module * SHS-5683: Blockquote implementation (#1575) * feat(shs-5683): blockquote implementation * feat(shs-5683): blockquote implementation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
READY FOR REVIEW
Summary
Update blockquotes in wysiwygs
Need Review By (Date)
07/12
Urgency
medium
Steps to Test
Text Areacomponent and add aBlockquoteTestimonialcomponent too to confirm it still look the samePR Checklist