react-new-window icon indicating copy to clipboard operation
react-new-window copied to clipboard

Option to disable centering

Open reminjp opened this issue 6 years ago • 2 comments

I am so grateful for this package. Now I want to set features.left and features.top myself. How about skipping centering if left and top is set? https://github.com/rmariuzzo/react-new-window/blob/2417b08352f34f193cc5e69524379fb70ccecf5b/src/NewWindow.js#L73-L77

or adding new option to center prop? https://github.com/rmariuzzo/react-new-window/blob/2417b08352f34f193cc5e69524379fb70ccecf5b/src/NewWindow.js#L177

reminjp avatar Jul 16 '19 15:07 reminjp

Unless somebody else has already done it, I'd like to try writing a pull request for this. My coworkers and I need something like it, too.

andrewsantarin avatar Jul 17 '19 02:07 andrewsantarin

@rdrgn Looking into it now, and... well... I don't know. The component config defaults to center="parent", and is evaluated somewhere down the line with an if-elseif-else block.

https://github.com/rmariuzzo/react-new-window/blob/2417b08352f34f193cc5e69524379fb70ccecf5b/src/NewWindow.js#L19-L29

It looks like from the online example that the component is perfectly capable of demonstrating a "not center" case by setting center={false}. See the story source code. By all accounts, we should be able to apply our own offsets right now without further pull requests (I'll still write my commit because I don't agree with the current approach). If you don't want to wait for my commit to get merged, simply do:

<NewWindow
  features={{ width: 600, height: 480, top: 100, left: 100 }}
  center={false} // You need to explicitly say `false` for "top" and "left" to work!!
  onUnload={() => this.newWindowUnloaded()}
>
  <h4>👋 Hi, again!</h4>
</NewWindow>

Please note that I've just learned about this fact today.


This section's more for you, @rmariuzzo.

While it works as advertised, there are two things that bug me:

  1. This center={false} condition isn't immediately obvious to everyone, especially when you're using TypeScript (like me). The center prop is explicitly defined as either 'center' or 'parent', so, if I chose to go with false, TypeScript complains, and I have to deliberately ignore it.
  2. The fact that I'm being shoehorned into center="parent" is somewhat contrived. I'm working on extending a docked layout library which needs to preserve non-center offsets, so defaulting to center kind of works against it. I should be given the freedom to go center only when I say so.

andrewsantarin avatar Jul 22 '19 07:07 andrewsantarin