Skip to content
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

fix: #288 Fixed Snippet tabs overwriting other tabs when closed #352

Merged
merged 3 commits into from
Mar 23, 2017
Merged

fix: #288 Fixed Snippet tabs overwriting other tabs when closed #352

merged 3 commits into from
Mar 23, 2017

Conversation

redcom
Copy link
Contributor

@redcom redcom commented Mar 22, 2017

This should fix the issue for you.

@asmsuechan
Copy link
Contributor

@redcom Thanks so much for fixing! It almost behaves correct, but I find an error when I delete the rightmost tab.

f8523a4e2d9ab48e7c0e3c1e1255013b

SnippetNoteDetail.js?64bd:267 Uncaught TypeError: Cannot read property 'reload' of undefined

@redcom
Copy link
Contributor Author

redcom commented Mar 22, 2017

This is true. I'll have a look later today.

@asmsuechan
Copy link
Contributor

Thank you!

@redcom
Copy link
Contributor Author

redcom commented Mar 23, 2017

Coll. I submitted a fix. It should be good now. Can you tell me what do you use to produce those animated gifs that show the issue?

@asmsuechan
Copy link
Contributor

asmsuechan commented Mar 23, 2017

@redcom This works fine. Thank you! I use Gyazo to capture my screen 😄 It can take at most 6 sec.

BTW, you don't need to combine your commits into one. Because it's trouble when I rebase your branch.

note: this.state.note,
snippetIndex
this.setState(() => {
let snippets = this.state.note.snippets.slice()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you change let to const?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@redcom
Copy link
Contributor Author

redcom commented Mar 23, 2017

sure

@asmsuechan
Copy link
Contributor

I assume this is more readable.

  deleteSnippetByIndex (index) {
    const snippets = this.state.note.snippets.slice()
    snippets.splice(index, 1)
    const note = Object.assign({}, this.state.note, {snippets})
    const snippetIndex = this.state.snippetIndex >= snippets.length
      ? snippets.length - 1
      : this.state.snippetIndex
    this.setState({ note, snippetIndex }, () => {
      this.save()
      this.refs['code-' + this.state.snippetIndex].reload()
    })
  }

@redcom
Copy link
Contributor Author

redcom commented Mar 23, 2017

It looks like so.

@asmsuechan
Copy link
Contributor

Cool! LGTM

@asmsuechan asmsuechan merged commit 6904c19 into BoostIO:master Mar 23, 2017
@redcom
Copy link
Contributor Author

redcom commented Mar 23, 2017

Thanks for accepting my commiting

@sota1235 sota1235 mentioned this pull request Apr 22, 2017
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.

None yet

2 participants