Conversation
connectorType was derived from the RTK Query result, which returns undefined once pending is cleared (skip: true). The modal condition active && connectorType would always be falsy, so the modal never opened in production. Store connectorType directly in active state so the modal renders independently of the skipped query.
…ig type If openVersionModal was called with a non-SaaS connection key, the effect condition never fired and pending stayed set indefinitely, keeping the hook subscribed to that connection key. Now pending is always cleared once connection resolves, regardless of whether a connectorType was found.
The clickable version badge was using a <span> with onClick, requiring two jsx-a11y eslint-disable comments. Replace with a <button> when connectionKey is present (keyboard-accessible, correct semantics) and a plain <span> when no connectionKey is given (non-interactive).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
| export interface State {} | ||
| const initialState: State = {}; | ||
|
|
||
| export interface SaaSConfigVersionResponse { |
There was a problem hiding this comment.
Is this a bespoke type or does it correspond directly to something on the backend (i.e. can it be autogenerated)?
There was a problem hiding this comment.
You're correct, it can be a model generated from the backend with the new skill. I was not aware of it so i didn't included it initally 😅
There are a some of other Models that got generated aswell, ignoring them for now.
| ver, | ||
| onClick, | ||
| }: { | ||
| ver: string; |
There was a problem hiding this comment.
Nit: prefer a more verbose variable name here, maybe versionIdentifier or similar.
| } | ||
| if (datasetError) { | ||
| return ( | ||
| <Text className="text-sm text-red-500"> |
There was a problem hiding this comment.
We don't use Tailwind for text colors so as to match the Ant palette, you can make red text by passing type="danger" as a prop here or type="secondary" for gray.
| ver: string; | ||
| onClick?: (e: React.MouseEvent<HTMLButtonElement>) => void; | ||
| }) => | ||
| onClick ? ( |
There was a problem hiding this comment.
UX-wise, clicking on this showing the version details has pretty poor discoverability-- maybe we can add an icon or tooltip?
There was a problem hiding this comment.
Using a tooltip, checking how it behaves 👍🏽
jpople
left a comment
jpople
left a comment
There was a problem hiding this comment.
Thanks for the changes, looks good!
2 participants
Ticket ENG-567
Description Of Changes
Adding Frontend Changes for Version History Modals. These would include the history of SaaS configs, both out of the box integrations and custom integrations, so the customers can have a better time checking their versions, and support can check what type/version is causing problems in case we have to check their systems
Code Changes
Regarding the side effect:
Changing away from
anymade TypeScript visible to pre-existing type issues that were previously silenced. There are 9connection!non-null assertions inside the hook that are now technically type-unsafe — though they are safe at runtime because the sole caller ([id].tsx) wraps the tab render with!!connection &&.The one assertion that did produce a compiler error:
This was fixed in this PR by converting
null → undefinedat the call site.Steps to Confirm
Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works