Skip to content

Fix: Tab not showing after refresh + store unsaved changes#1164

Merged
mykalmax merged 5 commits intomainfrom
fix-tab-not-showing-after-refresh
Jul 18, 2023
Merged

Fix: Tab not showing after refresh + store unsaved changes#1164
mykalmax merged 5 commits intomainfrom
fix-tab-not-showing-after-refresh

Conversation

@mykalmax
Copy link
Copy Markdown
Contributor

No description provided.

@mykalmax mykalmax force-pushed the fix-tab-not-showing-after-refresh branch from 67b9934 to 82f1b75 Compare July 17, 2023 23:20
@mykalmax mykalmax requested a review from vchan July 18, 2023 16:05
Comment on lines 240 to 243
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.log('restoreEditorTabsFromSaved', {
storedTabsIds,
storedTabsId,
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So storedTabsId can equal id or content? Shouldn't it only ever be able to equal id?

Comment thread web/client/src/context/editor.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's a bit confusing when id can sometimes be content and sometimes be id.

Comment thread web/client/src/context/editor.ts Outdated
Comment on lines 125 to 127
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So this new output tab won't always have content set?

@mykalmax
Copy link
Copy Markdown
Contributor Author

We have 2 types of tabs, from selecting a File or Custom SQL, if File has no unsaved changes , no need to store content cause we still calling API to get File content, but we need to match tab to file by id. So for File - id should always be there but content is optional. For Custom SQL we don't need id to match to anything so the important part is content. I was trying being clever but, you are right it is confusing and not necessary so I removed all that and just storing ids

Copy link
Copy Markdown
Contributor

@vchan vchan left a comment

Choose a reason for hiding this comment

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

Thank you for explaining the different types of tabs.

Comment thread web/client/src/library/pages/ide/IDE.tsx Outdated
@mykalmax mykalmax force-pushed the fix-tab-not-showing-after-refresh branch from e6bfd2d to e5ae2c9 Compare July 18, 2023 21:13
@mykalmax mykalmax merged commit 7e6af83 into main Jul 18, 2023
@mykalmax mykalmax deleted the fix-tab-not-showing-after-refresh branch July 18, 2023 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants