Skip to content
This repository was archived by the owner on Sep 4, 2024. It is now read-only.

Multiple tabs#1071

Merged
batpad merged 3 commits intodevelopfrom
feature/multiple-tabs
Apr 13, 2020
Merged

Multiple tabs#1071
batpad merged 3 commits intodevelopfrom
feature/multiple-tabs

Conversation

@geohacker
Copy link
Copy Markdown
Collaborator

Addresses #1009. Should be merged after IFRCGo/go-api#650
@necoline - let me know what you think, I think the way I'm managing the tabs in the state could probably be more elegant.

One thing I'm struggling with which we should absolutely fix is this: https://github.com/IFRCGo/go-frontend/blob/feature/multiple-tabs/app/assets/scripts/views/emergency.js#L116 -- if we do this on componentDidMount, the tabs aren't populated yet so it always resolves to #details.

cc @batpad

@geohacker geohacker requested a review from necoline April 1, 2020 15:54
@necoline
Copy link
Copy Markdown
Contributor

necoline commented Apr 1, 2020

Should we render all tab header and panels (aside from the default tab) in a child component?

@geohacker
Copy link
Copy Markdown
Collaborator Author

@necoline I'm a bit hesitant to rework this into child components. I tried real quick and the way Tabs are defined in a TabsList etc seem a bit tricky to get right if we pull into child components.

I just added a 0 set timeout after the tabs are registered and that seem to work well for users arriving at invalid hashes. I don't love this approach, but if you can think of better a way let's do that. Thanks!

cc @batpad

@geohacker geohacker force-pushed the feature/multiple-tabs branch from 9a3d44e to e8eff13 Compare April 7, 2020 11:43
@batpad batpad merged commit 4211a71 into develop Apr 13, 2020
@batpad batpad mentioned this pull request Apr 15, 2020
7 tasks
@batpad batpad deleted the feature/multiple-tabs branch September 3, 2020 15:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants