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

[client] Minor client bugfixes #886

Merged
merged 4 commits into from
May 25, 2022
Merged

Conversation

ix5
Copy link
Member

@ix5 ix5 commented May 25, 2022

Checklist

  • All new and existing tests are passing
  • (If adding features:) I have added tests to cover my changes
  • (If docs changes needed:) I have updated the documentation accordingly.
  • I have added an entry to CHANGES.rst because this is a user-facing change or an important bugfix
  • I have written proper commit message(s)

What changes does this Pull Request introduce?

  • js: embed: Make Postbox sibling, not child of #isso-root
  • js: embed: Catch nonexistent hash selectors
    We do not want to crash the whole app if a selector is not present.
  • [nit] js: api: fetch: Remove 404 handling
    Isso returns a JSON response with total_replies=0 instead of a 404 since version 0.12.3 (see troubleshooting, PR Return 200 with empty array when there are no comments #301 #565)
  • tests: comments: Add test for HTTP 200 on empty db

Why is this necessary?

State previously, on 0.12.5:

<#isso-thread>
    <h2>
    <.postbox>
    <#isso-root>

Then, because configs needed to be fetched before rendering the postbox because of: 02f3ea0
This was the state in 0.12.6:

<#isso-thread>
    <h2>
    <#isso-root>
    <.postbox>

But that left the order of #isso-root and the postbox wrong, the box was displayed after the comment list

The hotfix release 0.12.6.1: 861f64f tried to rectify this, but it resulted in the following
state:

<#isso-thread>
    <h2>
    <#isso-root>
        <.postbox>

This commit restores the state as it was:

<#isso-thread>
    <h2>
    <.postbox>
    <#isso-root>

ix5 added 4 commits May 25, 2022 11:15
State previously, on 0.12.5:
```
<#isso-thread>
    <h2>
    <.postbox>
    <#isso-root>
```
Then, because configs needed to be fetched before rendering
the postbox because of:
02f3ea0 "Have client read out shared settings from server"

This was the state in 0.12.6:
```
<#isso-thread>
    <h2>
    <#isso-root>
    <.postbox>
```

But that left the order of `#isso-root` and the `postbox`
wrong, the box was displayed *after* the comment list

The hotfix release 0.12.6.1:
861f64f "isso: js: embed: Insert Postbox before comments"
tried to rectify this, but it resulted in the following
state:
```
<#isso-thread>
    <h2>
    <#isso-root>
        <.postbox>
```

This commit restores the state as it was:
```
<#isso-thread>
    <h2>
    <.postbox>
    <#isso-root>
```

Note to future contributors: DOM polyfill `prepend()` will
insert the element before the first child, not before the
element itself!
We do not want to crash the whole app if a selector is not
present.
Isso returns a JSON response with `total_replies=0` instead
of a 404 since version 0.12.3 (see troubleshooting, PR isso-comments#565)
@ix5 ix5 added client (Javascript) client code and CSS bug labels May 25, 2022
@ix5 ix5 added this to the 0.13 milestone May 25, 2022
@ix5 ix5 merged commit 06dff17 into isso-comments:master May 25, 2022
@ix5 ix5 deleted the client-bugfixes branch May 25, 2022 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug client (Javascript) client code and CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant