DES-189
Our use of borders makes it very difficult to style a theme that looks
good with all states. Theme developers need a border to separate
comments from the content, but the current comments output includes
borders. It's possible to make it look ok in most of the time, but it's
difficult to work with. This change updates borders from the comments
CTA box with the following rules:
- Logged out without comments: no borders
- Logged out with comments: top border only
- Logged in without comments: no borders
- Logged in with comments: no borders
no issue
- moving the pagination button back to the top since it's not needed
now.
- Will move it to bottom in future along with additional comments enhancements.
ref MOM-135 MOM-211 MOM-213
- Bumped up the default comments count from 5 to 20 comments before
pagination kicks in.
- Moved the pagination (`Show X previous comments)` to bottom of the
page (soon to be reworded to `Show x more comments`)
- Updated tests and added a new helper for adding multiple comments easier.
closes https://linear.app/tryghost/issue/ENG-780
- the `admin-auth` frame was set up to log when any `message` event was handled but couldn't be parsed as JSON. For our own events that's fine but `message` events are also frequently triggered by browser extensions meaning the DevTools console log could become quite noisy for no reason
- removed the logging as it's noisy and not useful outside of development
closes https://linear.app/tryghost/issue/ENG-739
- `props.innerRef` in `<IFrame>` was inadvertently assumed to always exist, sometimes throwing an error on render when certain popups like the profile settings modal were opened resulting in the app crashing and the whole comments block disappearing
- added a guard to ensure the ref exists before trying to set it
- updated tests so the profile modal route is tested
closes ENG-711
When an Admin is authenticated in Comments-UI we only add moderation options to the displayed comments so we don't need to pre-emptively load the `admin-auth` iframe and make the `/ghost/api/admin/users/me/` request until some comments are actually visible.
- used `state.comments.length` property to defer rendering of the admin auth frame until comments have been fetched (after box is scrolled into view) and the count is > 0
closes ENG-681
There's no need to provide an `order` param with every request in Comments-UI if the API has default ordering that matches our requirements. The order param makes logs more noisy/harder to read than they need to be so we want to get rid of it.
- modified comments API input serializer to add a default order param to the browse and replies endpoints when none is provided
- removed order param from the requests that Comments-UI makes
refs https://linear.app/tryghost/issue/ENG-676/
Now we have the case that there is no filter param, the simple string
approach fails. Instead we build up a URLSearchParams object which
makes it easier to handle conditional params & stringify it at the end.
closes ENG-678
The comments block is typically shown at the bottom of a post so it doesn't make sense to eagerly fetch comments from the API when we don't know if the comments block will even be viewed. By lazy-loading the data only when the comments block comes into view we can reduce both data usage for visitors and load on the site.
- uses IntersectionObserver API to delay comments app initialisation until the comments block has scrolled into view
- updated all iframe-related components to forward a `ref` so we can use the `<iframe>` element reference inside the `App` component
refs https://linear.app/tryghost/issue/ENG-673
This means that the initial load of comments can be cached for everyone.
We also improve the timestamp which is used when fetching future comments,
instead of using a locally generated timestamp, we use the created_at of
the first comment loaded, this drastically improved the likelyhood that the
timestamp will be the same, meaning that it will hit the cache.
no ref
- includes run of `eslint --fix` to update changed classname ordering in Tailwind
---------
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Kevin Ansfield <kevin@lookingsideways.co.uk>
fixes GRO-34
fixes GRO-33
This is a revision of a previous commit, that broke the browser tests
because changes in the data generator (requiring bookshelf had side
effects).
This adds a new way to run all tests with enforced numeric ObjectIDs.
These numeric ids cause issues if they are used withing NQL filters. So
they surface tiny bugs in our codebase.
You can run tests using this option via:
NUMERIC_IDS=1 yarn test:e2e
Removed some defensive logic that could be explained by this discovered
issue.
fixes GRO-34
fixes GRO-33
This also adds a new way to run all tests with enforced numeric ObjectIDs.
These numeric ids cause issues if they are used withing NQL filters. So they
surface tiny bugs in our codebase.
You can run tests using this option via:
NUMERIC_IDS=1 yarn test:e2e
Also removed some defensive logic that could be explained by unquoted ids.
refs https://github.com/TryGhost/DevOps/issues/50
- when creating a TS config in our `eslint-plugin-ghost` dependency, I
only extended the recommended config, which left out a lot of
stylistic things we used to enforce in JS
- this fixes that by bumping the dependency to a version which extends
those shared configs, and fixes all the code that currently goes
against those rules
refs https://github.com/TryGhost/Product/issues/3504
- This adds support for translations, but doesn't yet translate every
possible string in the app.
- Only active if beta translations is enabled
refs https://github.com/TryGhost/Product/issues/3504
- App component now uses React hooks intead of React class component
- App is now written in TypeScript
- All JavaScript is now removed from the Comments-UI project
- Removed `PopupNotification` because these were never displayed
- Removed `action` from AppContext (never used)
- Moved options parsing out of `index.ts` into a separate utility file,
similar to the signup-form
- Improved reliability of some editor tests by always waiting for the
editor to be focused (was not always the case) + added an utility method
for this
refs https://github.com/TryGhost/Product/issues/3504
- When you are logged in as an admin, but not as a member, no buttons
showed (discovered in new e2e tests)
- Added E2E tests for admin actions
refs https://github.com/TryGhost/DevOps/issues/50
- `react-app` comes from `eslint-config-react-app`, which is a CRA package
- we're moving away from that so this commit switches the linting over
to a more recently updated plugin
- once that was removed, we started using a newer version of
`@typescript-eslint/eslint-plugin`, so there were plenty of
updates/exemptions to make
refs https://github.com/TryGhost/Team/issues/3504
This migrates comments-ui to TypeScript. Only `App.js` is left to
migrate, but since this isn't using hooks yet, it will need a bigger
rewrite so this will need to happen in a separate PR.
refs https://github.com/TryGhost/Team/issues/3504
- Sentry was never setup and we don't use it
- Styles have been moved to inline JS styles (no separate css file generated)
- App version was never used
- Improved current script tag detection
refs https://github.com/TryGhost/Team/issues/3504
- Removed Cypress. Tests will get replaced by Playwright
- Removed unused files and HTML files
- Updated scripts to work similar to Portal, Signup-Form
- Updated to pinned dependencies and removed unused dependencies
refs https://github.com/TryGhost/Team/issues/2221
- added guard to `formatNumber` helper used in the `<Count>` component via the `<ContentTitle>` component so a missing count prop is handled gracefully
- Changed all borders to work better in different background colours
- Same goes for main buttons like pagination for main and reply
- Same goes for any secondary text which should help
no issue
refs https://github.com/TryGhost/Team/issues/1780
This commit adds some Cypress tests as a POC to the project. If we decide to go with Cypress, we can add more tests to cover the rest of the codebase. The main reason to have a E2E framework is that some editor related UI is hard to test with the React tests because it uses JSDOM.
no issue
When the state of a comment changes (e.g., after a like) the relative time would update. This is not desirable because it looks glitchy. If the relative time doesn't update every second, then it is better to never update it after the initial render.
