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

[frogend] Settings refactor #1318

Merged
merged 33 commits into from
Jan 18, 2023
Merged

[frogend] Settings refactor #1318

merged 33 commits into from
Jan 18, 2023

Conversation

f0x52
Copy link
Contributor

@f0x52 f0x52 commented Jan 8, 2023

Progress towards refactoring the settings interface, almost finished now.
The codebase has grown kind of organically over time, writing newer/better ways to do things but not reflecting them in the existing sections. Most importantly this includes the use of RTK Query, and various refactors of the form field structure I yakshaved.

I'm standardizing on RTK Query everywhere, and self-designed form api/components that have a flexible hook structure to hold their data so they can be used in specialized usecases, or with a set of standardized form components in various types usage.

Goals:

  • Refactor internal API structure + form fields
    • Authentication
    • User profile
    • User settings
    • Admin settings
    • Admin actions (media cleanup)
    • Admin federation (overview, detail page)
    • Admin federation (bulk import/export)
    • Admin custom emoji (overview, detail, steal-this-look)
  • Remove lib/api completely, components/form-fields.jsx, old Redux reducers

NLnet Milestone 16: Settings interface refactor

@f0x52 f0x52 added the frontend Frontend-related stuff label Jan 8, 2023
@tsmethurst
Copy link
Contributor

Love how this is coming along, it looks a lot neater and easier to reason about!

@f0x52 f0x52 force-pushed the settings-refactor branch from 5ae77da to a4f0fb9 Compare January 13, 2023 23:57
@tsmethurst
Copy link
Contributor

oh mannnnnn it's coming along, i'm excited 👀

@f0x52
Copy link
Contributor Author

f0x52 commented Jan 17, 2023

All main goals are finished now, will review the CSS tomorrow and it can use some more testing!

@f0x52 f0x52 marked this pull request as ready for review January 17, 2023 20:42
@f0x52 f0x52 requested a review from tsmethurst January 17, 2023 20:42
@f0x52
Copy link
Contributor Author

f0x52 commented Jan 18, 2023

fixes #1253

Copy link
Contributor

@tsmethurst tsmethurst left a comment

Choose a reason for hiding this comment

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

Phew! Okay, it looks good! I have some comments as you can see, but they're mostly like 'nice' and small questions. I don't see anything here that gave me pause. Great work f0x! Feel free to merge it whenever you're ready :)

web/source/settings/admin/emoji/local/detail.js Outdated Show resolved Hide resolved
web/source/settings/admin/emoji/local/new-emoji.js Outdated Show resolved Hide resolved
web/source/settings/lib/form/form-with-data.jsx Outdated Show resolved Hide resolved
web/source/settings/lib/form/submit.js Show resolved Hide resolved
web/source/settings/lib/query/base.js Outdated Show resolved Hide resolved
web/source/settings/style.css Outdated Show resolved Hide resolved
@f0x52 f0x52 merged commit 9b139b6 into main Jan 18, 2023
@f0x52 f0x52 deleted the settings-refactor branch January 18, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Frontend-related stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants