-
Notifications
You must be signed in to change notification settings - Fork 8
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(client): modules page with basic features set #439
Conversation
wiring up logic
…zoom-to-module-or-verb
stopping for the night
…zoom-to-module-or-verb
…zoom-to-module-or-verb
Screen.Recording.2023-10-04.at.1.42.59.PM.mov |
export const ModulesRequests: React.FC<{ | ||
className?: string | ||
modules: Module[] | ||
}> = ({ className, modules }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good for now, but makes me think we might want a timeline component that we can pass in events to in the future so we don't have to duplicate all this code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed!
const VerbForm = ({ module, verb }: Props) => { | ||
const client = useClient(VerbService) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a VerbForm
component in the project today, would that work for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work for now. I want to style the Editor to look better in the panel later. I'll switch for now
interface MapValue { | ||
deploymentName: string | ||
id: string | ||
verbs: Set<VerbId> | ||
queriedVerbs: Set<VerbId> | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MapValue
seems a bit generic, is there a better name we could use here? Maybe like DeploymentVerbs
or something? I'm cool if this makes sense as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly I started generic and never swapped in something better to describe it. Let's rename.
const filters: EventsQuery_Filter[] = [] | ||
if (deployments.length) { | ||
filters.push(modulesFilter(deployments)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This component is named ModuleRequests
, so I just want to point out this is getting all events for a module. This would likely include Logs
, Calls
, Deployments
etc. Is that what you're looking for here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the moment yes. I need to site down and and figure out how to move to a more request trace based log like in the wireframes. Talk about it tomorrow?
note: there still some interactions and sizing on the svg I need to tighten up