Skip to content

FOUR-6996: Text Area Field Improvements #380

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

Merged
merged 4 commits into from
Feb 17, 2023
Merged

FOUR-6996: Text Area Field Improvements #380

merged 4 commits into from
Feb 17, 2023

Conversation

rodriquelca
Copy link
Contributor

@rodriquelca rodriquelca commented Jan 11, 2023

Issue & Reproduction Steps

Expected behavior:
Improve to text area field

Solution

  • Remove all the watch declarations if possible. Manly the ones that could use the deep: true flag
  • Remove all the computed properties if possible. Only the ones that really need to be reactive could be kept. It is better to evaluate then on data() create() * or mount()
  • Remove the v-bind="$attrs" if possible, it is better to define all the properties that the control can use including testing ones like data-cy
  • Avoid the use of transientData if possible.
  • v-if can reduce the reactivity (more than v-show) if it contains multiple reactive elements inside
  • Make sure v-for also includes a proper :key
  • Avoid the use of code like {... this.reactiveObject }e.g. { ...this.transientData } because it triggers the reactivity from all the properties of reactiveObject.

How to Test

  • Run npm run serve and start playing in the background.
    I would recommend linking this package to screen-builder and test it with the functionality there.

Related Tickets & Packages

Code Review Checklist

  • I have pulled this code locally and tested it on my instance, along with any associated packages.
  • This code adheres to ProcessMaker Coding Guidelines.
  • This code includes a unit test or an E2E test that tests its functionality, or is covered by an existing test.
  • This solution fixes the bug reported in the original ticket.
  • This solution does not alter the expected output of a component in a way that would break existing Processes.
  • This solution does not implement any breaking changes that would invalidate documentation or cause existing Processes to fail.
  • This solution has been tested with enterprise packages that rely on its functionality and does not introduce bugs in those packages.
  • This code does not duplicate functionality that already exists in the framework or in ProcessMaker.
  • This ticket conforms to the PRD associated with this part of ProcessMaker.

data() {
return {
objectOfAttrs: {
icon: this.$attrs.icon,
Copy link
Contributor

Choose a reason for hiding this comment

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

Where or how this property icon is used? I didn't find it in HTML TextArea elements nor Tinymce.

If this does not have effect consider to remove it.

refs:
https://www.tiny.cloud/docs/integrations/vue/#tinymcevuejsintegrationquickstartguide
https://www.w3schools.com/tags/tag_textarea.asp
https://www.w3schools.com/tags/ref_standardattributes.asp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the icon Attr was removed, because was not used

@caleeli caleeli self-requested a review January 13, 2023 19:54
@caleeli caleeli merged commit 451c61c into develop Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants