Draft
Conversation
Member
Author
|
@ChrisMarsh82 - is it still on your radar to give a first pass at this? I sent a couple reminders on mattermost both in April and May so just want to make sure it doesn't fall between the cracks |
Collaborator
|
@chrismclarke I did have a brief look at this but thought it would be better to get the updating variables out of the way before revisiting this as I feel that is an underlying structure change that could effect this. |
9ef770d to
84d82be
Compare
9efa388 to
4a0a96f
Compare
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
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.
PR Checklist
TODO
Core
data_changedtrigger)sortauthoring (likely inconsistent with items/data_items)markdownrow type (split PR?)Documentation
Future Follow-up
query_list(or similar) that run in the background and persist to fields (would require changes to how templates manage change-detection and re-render if field updates from outside of template)Description
Main Changes
Add new
data_queryrow type used to extract single data rows via querying and store to local variableComponent Sheet: component_data_query
Additional Changes
Create additional
markdownrow type used to directly render markdown with styling. This is more useful for things like tables or code blocks, which will be mostly unstyled when rendered within atextor other component.Add support for storing parameter_lists within data_lists in the correct format. This should also make it possible to refer to a different data ref to populate a row's parameter_list, reducing the need for copy-paste. e.g.
Author Notes
This deviates somewhat from the existing RFC, opting to provide an inline syntax for creating local
data_queryrows instead of global queries.The RFC will be updated accordingly once syntax finalised,
Review Notes
Dev Notes
In order to provide a
reffor pulling in adata_queryfrom an external source (e.g. data_list), I realised that data_lists would also need to make sure they process columns such asparamter_listin the same way as template. Hence the minor refactor to move parameter_list processing to the default parser. I don't anticipate this having any major knock-ons (not aware of people using parameter_lists in data_lists), but any diffs should show up following git sync (hard to review at the moment as debug content sheets currently outdated with git)The only other potential knock-on comes from the changes made to
template-variables.service.ts. I noticed it wasn't easily possible to pass json data from a data_list into a component directly (I think we have various component-level workarounds to manage things like processing lists). When trying to import aparameter_listcolumn from the data_list into the template row it would try use the evaluated data as a key to replace, resulting trying to assign values such as{ [{key: value_1}] : true}(which in console logs read{"[obj obj]": true}instead of spreading the object into the value{key : value_1}.Again, I don't think this is something that we try to do anywhere else, so wouldn't expect the changes to have any knock-ons.
One of the major challenges with implementing the querying system is handling mapping from JS logical operators to the syntax used by RXDB. I actually just threw the challenge at AI (using Gemini 2.5 flash) and I think it seemed to handle the immediate cases I had, but would appreciate a second pass on the code and tests to see if looks reasonable.
The
markdowncomponent addition was more just to provide an easier way in the component demo to display proposed authoring in a table format. I noticed the default markdown parser that gets called in components liketextdoesn't include any styling for things like tables or codeblocks, and couldn't find a good way to pass without interrupting the text component styles. So I think having a dedicated component makes sense to handle these types of cases, although again I just used AI to supply a broad list of styles so could do with some refining in the future (deciding what we do/don't want to couple more tightly with theming system) - I think likely this could be a follow-up issue as wouldn't really impact the rest of this PRGit Issues
Closes #
Screenshots/Videos
If useful, provide screenshot or capture to highlight main changes