Repository navigation
Permit Management - #57
Conversation
| setDurationSeconds, | ||
| handleSubmit, | ||
| } = usePermitForm({ | ||
| onSuccess: navigateToPermits, // TODO: also add toast here? |
There was a problem hiding this comment.
wdyt guys, should there be some toast or other kind of feedback upon creating a permit?
There was a problem hiding this comment.
Yes I agree, toast system would be very useful
| .filter((hash) => { | ||
| const serializedPermit = allPermits[hash]; | ||
| if (!serializedPermit) { | ||
| // TODO: revisit: right now, the way removal works, is that a key remains, but the value becomes undefined. https://github.com/FhenixProtocol/cofhesdk/blob/master/packages/sdk/permits/store.ts#L113 |
There was a problem hiding this comment.
i wonder if we should actually delete instead?
There was a problem hiding this comment.
The reason I didn't delete was because I thought that it doesn't play nicely in reactive environments, but that was a feeling and not something I checked. If delete doesn't break anything I'm happy to add it
There was a problem hiding this comment.
Either way if we are keeping it it should be return serializedPermit != null
| state.activePermitHash[chainId][account] = otherPermitHash; | ||
| } else { | ||
| if (!force) { | ||
| // TODO: is swallowed. Fix it |
There was a problem hiding this comment.
I'll see if I can deliver a fix in a separate PR
There was a problem hiding this comment.
Any idea why this is being swallowed, I cant track where this is happening
There was a problem hiding this comment.
apparently because permit removal is wrapped with result wrapper and catchFn is not provided, it wouldn't throw any error, and if you need to handle an error you need to look into the result
A bit counterintuitive. I wonder if there's a way to make it more intuitive
There was a problem hiding this comment.
Hmm, seems that the result wrappers aren't as useful as I hoped they would be. Maybe its worth considering removing them.
architect-dev
left a comment
There was a problem hiding this comment.
Generally not a fan of having multiple hooks.ts files sprinkled throughout the codebase. I'd prefer to have a single hooks folder with everything contained. I'm seeing a lot of '../../../../../../hooks/useCopyFeedback' stuff which I don't like. Can be refactored later though before it goes into master.
| setDurationSeconds, | ||
| handleSubmit, | ||
| } = usePermitForm({ | ||
| onSuccess: navigateToPermits, // TODO: also add toast here? |
There was a problem hiding this comment.
Yes I agree, toast system would be very useful
|
|
||
| // React to dynamic size changes inside the active page (e.g., toggling fields) | ||
| useEffect(() => { | ||
| if (!showPopupPanel) return; |
There was a problem hiding this comment.
Is it necessary? The content panel isn't able to dynamically resize based on the child content?
| .filter((hash) => { | ||
| const serializedPermit = allPermits[hash]; | ||
| if (!serializedPermit) { | ||
| // TODO: revisit: right now, the way removal works, is that a key remains, but the value becomes undefined. https://github.com/FhenixProtocol/cofhesdk/blob/master/packages/sdk/permits/store.ts#L113 |
There was a problem hiding this comment.
The reason I didn't delete was because I thought that it doesn't play nicely in reactive environments, but that was a feeling and not something I checked. If delete doesn't break anything I'm happy to add it
| .filter((hash) => { | ||
| const serializedPermit = allPermits[hash]; | ||
| if (!serializedPermit) { | ||
| // TODO: revisit: right now, the way removal works, is that a key remains, but the value becomes undefined. https://github.com/FhenixProtocol/cofhesdk/blob/master/packages/sdk/permits/store.ts#L113 |
There was a problem hiding this comment.
Either way if we are keeping it it should be return serializedPermit != null
| * - Copies provided text to clipboard and exposes transient "copied" state per key. | ||
| * - Useful for lists where each row has a copy action and needs quick feedback. | ||
| */ | ||
| export function useCopyFeedback(defaultDurationMs: number = 2000) { |
There was a problem hiding this comment.
cool system, haven't ever done it this way myself but it makes sense 👍
There was a problem hiding this comment.
did you hear, @copilot ? Good job
| state.activePermitHash[chainId][account] = otherPermitHash; | ||
| } else { | ||
| if (!force) { | ||
| // TODO: is swallowed. Fix it |
There was a problem hiding this comment.
Any idea why this is being swallowed, I cant track where this is happening
|
@alexshchur I've opened a new pull request, #58, to work on those changes. Once the pull request is ready, I'll request review from you. |
…into feat/APPS-109-permit-management
|
@architect-dev , seem to be 2 distinct issues here:
is it better for you like this?
I came up with an alias here. Wdyt? |
…into feat/APPS-109-permit-management
This pull request introduces a new "Permits" feature to the floating button UI in the React package, adds supporting components, and improves code modularity and maintainability. The main changes include new pages for viewing, generating, and receiving permits, integration of these pages into the floating button navigation, and the addition of reusable UI components for permit creation. It also refactors imports for consistency and adds dependencies for SVG handling.
New Permits Feature Integration:
PermitsListPage,GeneratePermitPage, andReceivePermitPage, and integrated them into the floating button navigation and context (FnxFloatingButtonContext.tsx,ContentSection.tsx,MainPage.tsx,pages/index.ts). [1] [2] [3] [4] [5]Reusable UI Components for Permit Creation:
ExpirySection,NameSection,ReceiverSection, andSelfToggleto improve code reusability and clarity. [1] [2] [3] [4]Floating Button UI Improvements:
ResizeObserver.Codebase Maintenance and Refactoring:
@/alias for hooks and utilities, improving consistency and maintainability across multiple files. [1] [2] [3]Development Dependency Updates:
@svgr/coreand@svgr/plugin-jsxtodevDependenciesinpackage.jsonto enable SVG component handling.