Scribble component #1569
Open
Scribble component #1569
Conversation
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
|
WIP, Looking for early feedback. |
rajatofficial
force-pushed
the
scribble-component
branch
from
4b5c8ac to
2ece48e
Compare
Lighthouse scores (desktop)
|
Lighthouse scores (mobile)
|
Accessibility Violations Found
|
Accessibility Violations Found
|
1 similar comment
Accessibility Violations Found
|
Lighthouse scores (desktop)
|
Lighthouse scores (mobile)
|
Accessibility Violations Found
|
2 similar comments
Accessibility Violations Found
|
Accessibility Violations Found
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1569 +/- ##
============================================
- Coverage 82.32% 82.26% -0.07%
Complexity 1000 1000
============================================
Files 108 109 +1
Lines 2603 2605 +2
Branches 370 370
============================================
Hits 2143 2143
- Misses 272 274 +2
Partials 188 188 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Lighthouse scores (desktop)
|
Lighthouse scores (mobile)
|
Accessibility Violations Found
|
Accessibility Violations Found
|
Accessibility Violations Found
|
Lighthouse scores (mobile)
|
Lighthouse scores (desktop)
|
Accessibility Violations Found
|
1 similar comment
Accessibility Violations Found
|
Accessibility Violations Found
|
Lighthouse scores (desktop)
|
Lighthouse scores (mobile)
|
Accessibility Violations Found
|
1 similar comment
Accessibility Violations Found
|
Accessibility Violations Found
|
sakshi-arora1
requested changes
May 13, 2025
| @@ -0,0 +1,49 @@ | |||
| /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | |||
| ~ Copyright 2024 Adobe | |||
There was a problem hiding this comment.
change all copyright to 2025
| * Returns the base64 encoded string of the scribble. | ||
| * | ||
| * @return Base64 encoded string representing the scribble | ||
| * @since com.adobe.cq.forms.core.components.models.form 1.0.0 |
| By placing the class names `cmp-adaptiveform-scribble__label` and `cmp-adaptiveform-scribble__questionmark` within the `cmp-adaptiveform-scribble__label-container` class, you create a logical grouping of the label and question mark elements. This approach simplifies the process of maintaining a consistent styling for both elements. | ||
|
|
||
| ## Replace feature: | ||
| We support replace feature that allows replacing Reset Button component to any of the below components: |
There was a problem hiding this comment.
should be replaced by nothing
| <customFormats | ||
| jcr:primaryType="nt:unstructured" | ||
| sling:resourceType="granite/ui/components/coral/foundation/form/multifield" | ||
| fieldDescription="The scribble formats that are allowed to be selected by an author." |
There was a problem hiding this comment.
Have we got these approved, doesn't make much sense
| The following are optional attributes that can be added to the component in the form view: | ||
| 1. `data-cmp-valid` having a boolean value to indicate whether the field is currently valid or not | ||
| 2. `data-cmp-required` having a boolean value to indicate whether the field is currently required or not | ||
| 3. `data-cmp-readonly` having a boolean value to indicate whether the field is currently readonly or not |
There was a problem hiding this comment.
we are not exposing readonly in dialog, keep it consistent with implementation
|
|
||
| $(document).on("dialog-ready", function() { | ||
| var $editDialog = $(EDIT_DIALOG); | ||
| if ($editDialog.length > 0) { |
There was a problem hiding this comment.
If we don't require this, don't create it
| } | ||
|
|
||
| .cmp-adaptiveform-scribble__questionmark { | ||
|
|
There was a problem hiding this comment.
inconsistent with implementation
| data-cmp-visible="${scribble.visible ? 'true' : 'false'}" | ||
| data-cmp-enabled="${scribble.enabled ? 'true' : 'false'}" | ||
| data-cmp-required="${scribble.required ? 'true': 'false'}" | ||
| data-cmp-readonly="${scribble.readOnly ? 'true' : 'false'}" |
There was a problem hiding this comment.
this was removed in dialog, remove it from here as well
There was a problem hiding this comment.
Do we have a usecase for readonly, discuss that first
|
|
||
| <div data-sly-call="${errorMessage.errorMessage @componentId=scribble.id, bemBlock='cmp-adaptiveform-scribble'}" data-sly-unwrap></div> | ||
| <div class="cmp-adaptiveform-scribble__container" tabindex="0" role="dialog" aria-label="pleaseSignText"> | ||
| <div class="cmp-adaptiveform-scribble__header">Please sign here</div> |
There was a problem hiding this comment.
this is not localized
| <div class="cmp-adaptiveform-scribble__canvases"> | ||
| <div class="cmp-adaptiveform-scribble__signcanvases" style="display:inline-block; vertical-align:top;"> | ||
| <canvas class="cmp-adaptiveform-scribble__canvas"></canvas> | ||
| <input type="text" class="cmp-adaptiveform-scribble__keyboard-sign-box" name="signatureText" placeholder="Type Your Signature Here"> |
There was a problem hiding this comment.
why are we removing placeholder from dialog, when we can use it here. Also the one you have hardcode is not localized
Lighthouse scores (desktop)
|
Lighthouse scores (mobile)
|
Accessibility Violations Found
|
1 similar comment
Accessibility Violations Found
|
| scribbleContainerPanel: `.${Scribble.bemBlock}__panel`, | ||
| keyboardSignBox: `.${Scribble.bemBlock}__keyboard-sign-box`, | ||
| scribbleControlPanel: `.${Scribble.bemBlock}__controlpanel`, | ||
| geoCanvasRight: `.${Scribble.bemBlock}__geocanvasright`, |
There was a problem hiding this comment.
why geoCanvasRight and not just geoCanvas?
| clearControl: `.${Scribble.bemBlock}__control-clear`, | ||
| scribbleTextControl: `.${Scribble.bemBlock}__control-text`, | ||
| scribbleMessage: `.${Scribble.bemBlock}__control-message`, | ||
| clearSignButton: `.${Scribble.bemBlock}__clearsign-yes`, |
There was a problem hiding this comment.
just clearsign would do rather than clearsign-yes
| scribbleTextControl: `.${Scribble.bemBlock}__control-text`, | ||
| scribbleMessage: `.${Scribble.bemBlock}__control-message`, | ||
| clearSignButton: `.${Scribble.bemBlock}__clearsign-yes`, | ||
| cancelClearSignButton: `.${Scribble.bemBlock}__clearsign-no`, |
There was a problem hiding this comment.
just cancel-clearsign would do rather than clearsign-no
| clearSignButton: `.${Scribble.bemBlock}__clearsign-yes`, | ||
| cancelClearSignButton: `.${Scribble.bemBlock}__clearsign-no`, | ||
| clearSignContainer: `.${Scribble.bemBlock}__clearsign-container`, | ||
| scribbleSubmitControl: `.${Scribble.bemBlock}__control-submit`, |
There was a problem hiding this comment.
repeated in saveCanvas
| scribbleSubmitControl: `.${Scribble.bemBlock}__control-submit`, | ||
| scribbleBrushList: `.${Scribble.bemBlock}__brushlist`, | ||
| signCanvases: `.${Scribble.bemBlock}__signcanvases`, | ||
| saveCanvas: `.${Scribble.bemBlock}__control-submit`, |
There was a problem hiding this comment.
why not just _save rather than control-submit
| scribbleContainerPanel: `.${Scribble.bemBlock}__panel`, | ||
| keyboardSignBox: `.${Scribble.bemBlock}__keyboard-sign-box`, | ||
| scribbleControlPanel: `.${Scribble.bemBlock}__controlpanel`, | ||
| geoCanvasRight: `.${Scribble.bemBlock}__geocanvasright`, |
There was a problem hiding this comment.
we can also pre-append button in case functionality is of button
| <h1 class="cmp-adaptiveform-scribble__clearsign-title">Erase Current Signature?</h1> | ||
| <div class="cmp-adaptiveform-scribble__clearsign-content" class="msgBoxTypenull"> | ||
| <div class="cmp-adaptiveform-scribble__clearsign-message"> | ||
| This will permanently remove the signature you've drawn. Do you wish to proceed and begin again? |
| This will permanently remove the signature you've drawn. Do you wish to proceed and begin again? | ||
| </div> | ||
| <div class="cmp-adaptiveform-scribble__clearsign-panel"> | ||
| <button type="button" id="msgBox_No" class="cmp-adaptiveform-scribble__clearsign-no">Cancel</button> |
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.
No description provided.