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

feat(fee-breakdown): Initial Editor UI #4030

Closed
wants to merge 1 commit into from

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Dec 3, 2024

What does this PR do?

  • Updates the Editor UI to illustrate how the fee breakdown works
  • Very open to suggestions here! To me something along these lines of "show not tell" is a bit more digestible / explainable and allows room for some rows to be added (e.g. service charge), or an input to be added (for configuration)
  • Just UI behind a feature flag, no functional changes here

Questions

I've hardcoded application.fee as a prefix here, but this is a dynamic key.

In reality we know that all active services (apart from one) currently pass through the osl/fee-calculator flow which rely on this variable.

Here's the question - should we keep this dynamic, or make the field disabled in the Pay component and always set to application.fee (after double checking services)? I think making it static leans more towards the "SetFee" component approach we discussed, but isn't without downsides. Maybe one for the dev call!

image

<TableCell>Reduction</TableCell>
<TableCell align="right" sx={{ display: "flex", alignItems: "center" }}>
<HelpTooltip text="This is the sum of all reductions applied to the application fee. The list of reductions will be listed below" />
<DataField>- £ (calculated - payable)</DataField>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this shorthand for application.fee.calculated - application.fee.payable is that clear?

The "full" version pretty much always overflows and feels less clear even though the above is not technically correct.

image

Copy link

github-actions bot commented Dec 3, 2024

Pizza

Deployed f20ef6e to https://4030.planx.pizza.

Useful links:

@DafyddLlyr DafyddLlyr marked this pull request as draft December 4, 2024 15:07
@DafyddLlyr
Copy link
Contributor Author

Closing in favour of a list based approach and not a illustrative table / fee breakdown. Will reference this PR as prior art!

@DafyddLlyr DafyddLlyr closed this Dec 4, 2024
@DafyddLlyr DafyddLlyr deleted the dp/fee-breakdown-editor-ui branch December 4, 2024 16:15
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.

1 participant