POC new component for copying reports #23308
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
|
@caddoo Could some of the team look over this POC draft PR and give some initial feedback on the approach? They can use the Jira ticket PG-4019 as reference. |
|
@snake14 LGTM, with the caveat that I'm new to the codebase. I've only looked at it from a PoC and structural point of view, and haven't reviewed any code in depth, or tested. |
Appreciate the POC draft approach @snake14 . As this is a POC, i will comment just on the code, the interface for other developers and also a bit on the UI. Some feedback items ( I don't know how much are already decisions, so will just mention all ). On the interface for the component. I had to try and use it myself to try and understand how it works and came up with: Maybe naming a component other than The property called
In terms of UI, it feels like an anti-pattern to have disabled submit buttons, the user doesn't know what to do in order to make it active. Usually they would be able to click it to see what is required, but again just opinion. |
|
Thank you @caddoo . Your example usage looks pretty close to what I was using to test. 👍
I'm definitely open to different name ideas. I simply went with
Yes. It's supposed to be a translated item. I could also adjust it to allow a translation key. I kept it somewhat generic because it's used by 3 translations in the component, including the title. Any occurrences of 'report' in the example screenshot I provided in the description are using the
I could use some feedback around this. I thought about using an interface, but I also want to allow each usage of the component to provide the data required to make a copy. For example, one report might only need the report ID and the destination site ID while others might want to select additional options. Leaving the
That's a fair point. I was thinking about the save button of some of our update forms starting out disabled until the form is 'dirty', but this is a different use case. I can change the submit button to be enabled until validation fails. |
* Implement the server-side interface
Description:
This is a POC for a new component which can be used throughout the system for implementing the ability to copy reports and other entities, like goals, funnels, and so on. It uses some events to alert the parent of certain actions, such as validation. Some changes were made to SiteSelector to allow somewhat more configurability.
For right now, we're simply allowing one site and not allowing roll-up sites. We plan to add the ability to select more than one site at a time and come up with a better mechanism for determining whether roll-up sites should be allowed based on what is being copied.
Here is a test implementation:
The word 'report' is the default value for what is being copied and can be overridden using a prop. The div with the test form inputs is the slot defined by the parent view and is scrollable when it exceeds the available height. The copy button is disabled until validation passes.
Review