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

RMIZ's future: React 16 & beyond RFC #143

Closed
rpearce opened this issue Dec 7, 2018 · 7 comments
Closed

RMIZ's future: React 16 & beyond RFC #143

rpearce opened this issue Dec 7, 2018 · 7 comments
Assignees

Comments

@rpearce
Copy link
Owner

rpearce commented Dec 7, 2018

Intro

With the latest React version and React's future as a whole, some things have come to light that need changing, and this has me thinking that it's time for an overhaul on this project.

Current State

While the project works fine as it is right now, here are some observations:

  • mixing the controlled vs uncontrolled component modes makes for some very messy code, and it results in some people requiring in code that has nothing to do with how they use the component
  • current lifecycle methods need to be changed & future-proofed (as well as one can)
  • there exists a currently seemingly insurmountable issue (Handle scroll event using a container element #136) that is something I'm afraid I can't address with how things are built right now and feel I can't rip everything apart because...
  • there are no tests (Write tests for this #55)
  • keyboard accessibility is a bit hacky at the moment

The Future?

Split Controlled & Uncontrolled Components

For this, I'm thinking either 1 of 2 things:

  • offer up different components to import or
  • go the monorepo route and offer them up as different packages

I could use some feedback on this.

All the "business logic" around what makes these components tick would naturally be extracted, shared and tested. Unlike input components, which can be controlled or uncontrolled (given you provide a value or not), developers know from the get-go how they're going to use this component, so I'm of a mind to have them choose from the start.

Updates for future React

Testing

I've gotten a lot better about testing React components since this was initially published and updates made. I might as well use this as an opportunity to test all the things instead of relying on Storybook.

Keyboard a11y

Not much to say here other than it seems a bit wonky across browsers. Can address more fully if it's baked in from the get-go.

General Cleanliness

Parts of this library are quite difficult to follow and are hacky because it's what I had to do to make the library work. That said, I think this could improve the quality of the code (and dependability with tests) and make it easier for others to contribute.

Portability Across Frameworks / Languages

I'd love to identify patterns and code blocks that can be used as inspiration for porting this to

  • vanilla JS
  • elm
  • purescript
  • vue (which I know nothing about)
  • ...?
    if I or someone else would like to.

Docs

Small point, but I'm going to move the documentation to separate markdown files and keep the README clean and to the point:

  • API.md
  • CONTRIBUTING.md
  • whatever else is needed

Thoughts? I'm asking all of you because you have contributed in this project in some way and should have a say:
cc @cbothner @jeremybini @ismay @rajit @rsaccon @wtfdaemon @aswinckr @alexshelkov @Snowsoul @kendagriff @hippodippo @oyeanuj @ludwigfrank @serfgy @babyPrince

@cbothner
Copy link
Contributor

I’m in favor of exporting multiple components from the same package, rather than splitting into two packages. It’s likely a user will start with the stateful component and then grow into needing the lower level controlled component. My thinking is that it’s lower friction and more discoverable to change the import at the top of the file rather than package.json.

@rpearce
Copy link
Owner Author

rpearce commented Dec 10, 2018

@cbothner thank you for the feedback!

@rpearce
Copy link
Owner Author

rpearce commented Dec 28, 2018

Update: I've now broken ground on this. Here we go... Working from the rewrite branch

@rpearce
Copy link
Owner Author

rpearce commented Feb 20, 2019

Update: This has been paused as I've been having hand+wrist+forearm pain and need to deal with that before doing this outside normal working hours.

Good news is that we'll be using latest & greatest React & hooks!

@rpearce
Copy link
Owner Author

rpearce commented May 25, 2019

@spencerfdavis is going to be giving me a hand with this. Welcome to the project, @spencerfdavis, and I look forward to getting some things rolling together over the coming months!

@rpearce
Copy link
Owner Author

rpearce commented Sep 29, 2019

Update: I've been putting a bit of work into the rewrite and am having a meeting with @spencerfdavis on Monday to figure out what path we want to go down with regard to some things.

We're going to

  • plan everything that needs doing out (and how ... this is interesting)
  • create a project board
  • break things into issues and place them on that board
  • use Rewrite prototype #159 as the main rewrite branch until we have something that solves all outstanding problems & we determine a good API. Note: what is there right now is a prototype for ideas around what we're trying to accomplish.

The ultimate goals are:

  • accessibility
  • simplify, be smarter, much less code
  • 100% test coverage
  • easily work with the styles (that rewrite branch has me doing styleInject via rollup, but I've come to learn that content security policies do not jive with that sort of thing well...we may just output a .css file...TBD)
  • ensure all outstanding issues are handled (that includes mobile)

There are also a few other folks who have expressed interest in contributing (thanks, Hacktoberfest!), so more to come on that, as well.

@rpearce
Copy link
Owner Author

rpearce commented Oct 6, 2019

The prototype for the rewrite is out on the master branch (master is dev for now), and it needs a lot of work. We'll be tracking its progress in https://github.com/rpearce/react-medium-image-zoom/projects/1, so I'm going to close this issue.

@rpearce rpearce closed this as completed Oct 6, 2019
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

No branches or pull requests

3 participants