Disabling search bar

[garciafdezpatricia]

Disable search Box

Disabling search box when there are no access grants to show

User testing instructions

Commit checklist

• All acceptance criteria are met.
• Includes tests to ensure functionality, accessibility and prevent regressions, including E2E tests for supported browsers.
• Relevant documentation, if any, has been written/updated.
• The changelog has been updated, if applicable.
• A changeset version has been created, if applicable.
• Meets Inrupt coding standards and adheres to commit conventions.

Design requirements checklist

• Meets Inrupt API Design and UX standards
• Is responsive to our documented minimum supported screen size + resolution
• Code is extensible by external contributors or anyone forking

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
authorization-management-component	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 13, 2024 2:46pm

[NSeydoux]

I would prefer a different approach where the ListAgentsWithVCs component is given a more generic callback that takes as an argument the number of agents. The useEffect in ListAgentsWithVCs would keep the same dependency, and call that onAgentUpdate callback with the agent count (defaulting to 0). The logic to check if the agent count is zero could then be lifted into the AuthenticatedRoute.

[NSeydoux]

Suggested change

	if (agents !== undefined && agents.length > 0) {
	if (onNotEmptyAgents) {
	onNotEmptyAgents();
	}
	}
	if (agents !== undefined && agents.length > 0 && typeof onNotEmptyAgents === "function") {
	onNotEmptyAgents();
	}

[garciafdezpatricia]

Why is it better in this case to lift the check and disabling of the search bar to the AuthenticatedRoute? I thought that component was "responsible" of checking the authentication of the session and return the correct content in each case.

[NSeydoux]

As I understand the current architecture, AuthenticatedRoute is the component managing the display of the search bar. The reason I'm suggesting to lift the check up and have a more generic callback provided to ListAgentsWithVCs is because with the current approach, you can only have a trigger when there are agents. But if the parent needs to change its behavior if there are no agents, or even more specifically if there is a given number of agents (let's say you want to change display on a large number of agents compared to a few), you would not be able to do that because the current callback is hardcoded to non-zero.

[NSeydoux]

Replace all occurences opf AutghenticatedRoute by index because I don't know how to read github diffs anymore

[NSeydoux]

Some superficial suggestions, but overall looks good to me.

[NSeydoux]

Suggested change

	onAgentUpdate={(agentCount) => {
	agentCount === 0
	? setDisableSearch(true)
	: setDisableSearch(false);
	onAgentUpdate={(agentCount) => {
	setDisableSearch(agentCount === 0)

Often, when you have a test and then return true or false, you can return the test directly :)

[NSeydoux]

Suggested change

	let agentsCount = 0;
	if (agents !== undefined) {
	agentsCount = agents.length;
	}
	onAgentUpdate(agentsCount);
	onAgentUpdate((agents ?? []).length);

I don't have a strong opinion on which one we should use, feel free to keep the one you find easier to read and understand.