Skip to content

ILR Tab menu changes#14055

Open
FinnIckler wants to merge 4 commits intothewca:mainfrom
FinnIckler:tab-menu-changes
Open

ILR Tab menu changes#14055
FinnIckler wants to merge 4 commits intothewca:mainfrom
FinnIckler:tab-menu-changes

Conversation

@FinnIckler
Copy link
Copy Markdown
Member

Comment on lines +31 to +38
function parseActivityCodeOrNull(path: string) {
try {
const { eventId } = parseActivityCode(path);
return eventId;
} catch {
return null;
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nope. No try-catch. Not at all.
What is causing parseActivityCode to raise an exception and how can we detect these edge cases before passing the data into parseActivityCode?

height="fit-content"
position="sticky"
minWidth="fit-content"
width="3xs"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did the previous not work? I think min-width makes a lot of sense with respect to "making sure that long tab titles still at least fit the darn thing", perhaps you want to assing a max-width separately?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's because the content size changes when you open the submenus. So the UI gets jumpy. max-width doesn't change anything here

<Text
textStyle="bodyEmphasis"
asChild
maxW="44"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where did this magical constant come from? I know it is defined in Chakra, but why did you choose it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is from the previous version of the menu code that I accidentally deleted. I think it was the most fitting option for custom tabs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants