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

Teal bug fixes #13593

Merged
merged 17 commits into from May 3, 2024
Merged

Teal bug fixes #13593

merged 17 commits into from May 3, 2024

Conversation

aptkingston
Copy link
Member

@aptkingston aptkingston commented May 2, 2024

Description

This PR is a collection of fixes for issues discovered during building our app.

Fixes

  • Fix prompts in button action chains breaking event context for following actions
  • Rewrite how state keys are detected so they will detect usage inside form steps, form blocks, and any other "nested" component style settings
  • Add support for using _id and _rev of clicked rows when using a relationship as a grid datasource
  • Fix static formulas having no operator options
    image
  • Add support for using _id and _rev of clicked rows when using a data provider as a table data source, and that data provider is using a datasource plus source

Improvements

  • Made both settings of the "Prompt user" button action bindable (they were previously just static text)
  • Fixed tooltips for form step configuration controls
  • Improved grid/table ability to handle more types of error messages
  • Remove prettier warnings inside account portal coverage files
  • Remove eslint warning about personal config files (if you have one) by add the root key to our config
  • Refactor click_outside utils to both keep the current fix of not closing side panels when dragging outside, but also fix those annoying drag-outside closing events for good (selecting text inside a modal for example), and fix the current issue of side panels flashing when clicking between rows in grids

Addresses

@aptkingston aptkingston marked this pull request as draft May 2, 2024 10:40
@aptkingston aptkingston marked this pull request as ready for review May 2, 2024 14:35
@aptkingston aptkingston changed the title (WIP) Teal bug fixes Teal bug fixes May 2, 2024
@@ -92,7 +92,8 @@
}

const addStep = () => {
value = value.toSpliced($currentStep + 1, 0, {})
const newInstance = componentStore.createInstance(componentType)
value = value.toSpliced($currentStep + 1, 0, newInstance)
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of using an empty object, we now create a fake component instance for each form step. This is how we usually handle these settings, and makes it easier to use later with generic utilities because we have a real component type saved for which we can lookup the definition and settings.

_id: Helpers.uuid(),
_component: componentType,
_id: savedInstance._id || Helpers.uuid(),
_component: savedInstance._component || componentType,
Copy link
Member Author

Choose a reason for hiding this comment

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

Just re-using the saved properties since they will exist for all new steps after this change. Makes no difference but is how it would be written if we had always saved them.

}}
/>
</AbsTooltip>
<AbsTooltip text="Next step" noWrap>
Copy link
Member Author

Choose a reason for hiding this comment

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

Wrapping all of these in an AbsTooltip rather than the native tooltip prop of the action button so that the tooltips are rendered on top.

Copy link
Member Author

Choose a reason for hiding this comment

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

All nested types are now handled in a single generic way. The old code handled all of these cases apart from fieldConfiguration, and was also very duplicated. This is a single generic way to do this now for all these types of settings.

resolve(typeof next === "function" ? await next() : true)
if (typeof next === "function") {
// Pass the event context back into the new action chain
resolve(await next(eventContext))
Copy link
Member Author

Choose a reason for hiding this comment

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

We were just missing the event context param here to the following button actions. The rest is just moving around some code to make it more readable.

@@ -124,6 +124,7 @@
const fieldSchema = schemaFields.find(x => x.name === filter.field)
filter.type = fieldSchema?.type
filter.subtype = fieldSchema?.subtype
filter.formulaType = fieldSchema?.formulaType
Copy link
Member Author

Choose a reason for hiding this comment

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

The formula type was missing from the structure we pass into the lucene utils in shared core, which resulted in us being unable to detect static formulae.

Copy link
Contributor

@deanhannigan deanhannigan left a comment

Choose a reason for hiding this comment

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

LGTM! So many great fixes in here 🚀

@aptkingston aptkingston merged commit 5781a89 into master May 3, 2024
10 checks passed
@aptkingston aptkingston deleted the bug-crusher-9000 branch May 3, 2024 12:57
@github-actions github-actions bot locked and limited conversation to collaborators May 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants