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.

Sign up for GitHub

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

Jump to bottom

refactor: remix migration #3789

Open
wants to merge 149 commits into
base: master
Choose a base branch
from
Open

refactor: remix migration #3789

wants to merge 149 commits into from

Conversation

mariojsnunes
Copy link
Collaborator

@mariojsnunes mariojsnunes commented Jul 22, 2024 •
edited
Loading

PR Checklist

PR Type

What kind of change does this PR introduce?

  • Refactoring (no functional changes)
  • Build related changes
  • CI related changes
  • Documentation changes

What is the current behavior?

Frontend static files.

What is the new behavior?

  • We need a server for the frontend.
  • First steps towards server-side rendering.
  • This goal of this PR is to do the minimum changes necessary for Remix to work, the App is still rendered on the browser.

docs/remix.md for more details :)

TODO

  • update ci/cd scripts
  • fix unit and e2e tests
  • localStorage/sessionStorage/window usage might need a refactor

Does this PR introduce a breaking change?

  • Yes
  • No

Git Issues

Closes #

benfurber
benfurber reviewed Jul 22, 2024
View reviewed changes
Copy link
Member

@benfurber benfurber left a comment

Choose a reason for hiding this comment

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

Fun! Nice work!

packages/components/src/test/utils.tsx Outdated Show resolved Hide resolved
src/.eslintrc.json Show resolved Hide resolved
src/common/AuthWrapper.tsx Show resolved Hide resolved
src/pages/Layout.tsx Outdated Show resolved Hide resolved
src/pages/Question/question.routes.test.tsx Outdated Show resolved Hide resolved
src/routes/how-to.$slug.edit.tsx Outdated Show resolved Hide resolved
src/routes/academy.$.tsx Outdated Show resolved Hide resolved
src/routes/academy.$.tsx Outdated Show resolved Hide resolved
src/routes/questions.$slug._index.tsx Outdated Show resolved Hide resolved
src/routes/research.$slug.edit-update.$update.tsx Outdated Show resolved Hide resolved
@benfurber
Copy link
Member

Some meta tags aren't updating correctly on FF:
Screenshot 2024-09-23 at 13 36 43

@benfurber
Copy link
Member

On https://community-platform-pk.fly.dev/research/should-we-get-a-tractor-to-work-on-the-land#update_tnUSigHLpLa7hjI7SRuk-comment:G00gaXwslHCnxD78iVJ3:

Some images come through (e.g. all on this update)
Screenshot 2024-09-23 at 14 47 11

But none on this update (including mine):
Screenshot 2024-09-23 at 14 48 01

@benfurber
Copy link
Member

I couldn't upload an image to a research update:
Screenshot 2024-09-23 at 14 54 25

@benfurber
Copy link
Member

benfurber commented Sep 23, 2024 •
edited
Loading

Right favicon not pulled though:

Screenshot 2024-09-23 at 15 52 29

@benfurber
Copy link
Member

Screenshot 2024-09-25 at 13 11 10

@mariojsnunes mariojsnunes removed the Review allow-preview ✅ Has received manual check for malicious code and can be safely built for preview label Sep 25, 2024
@mariojsnunes mariojsnunes marked this pull request as ready for review September 25, 2024 18:32
@mariojsnunes mariojsnunes requested a review from a team as a code owner September 25, 2024 18:32
benfurber
benfurber reviewed Sep 26, 2024
View reviewed changes
docs/remix.md
Comment on lines +25 to +28
- Current issues
- unit and e2e tests are not passing
- localStorage/sessionStorage/window usage might need a refactor
- DevSiteHeader (minor) -> needs a navigation/refresh after changing theme
Copy link
Member

Choose a reason for hiding this comment

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

Are these issues still valid?

@benfurber
Copy link
Member

Screenshot 2024-09-26 at 11 10 07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benfurber benfurber benfurber left review comments

Assignees
No one assigned
Labels
Backend Documentation Mod: HowTo 📰 Mod: Maps 🗺 Mod: Research 🔬
Projects
Community Platform
Status: No status
Core Team
Status: In progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mariojsnunes @benfurber @davehakkens @goratt12