Logical offsets for start/end are not intuitive

[ling1726]

Note that this is not about rtl support -> that is actually awesome 🚀.

Just some examples to illustrate what I'm talking about

Popper v2: https://codesandbox.io/s/nervous-bird-xg963v?file=/src/index.js
Floating UI: https://codesandbox.io/s/sad-hellman-drowyp?file=/src/index.js:451-465

I've noticed that offset axis now can invert based on alignment. This is not very intuitive since offset now needs to be correctly set based on the the placement that is chosen, i.e. the x,y axis changes orientation.

This is also a huge change from Popper. I can't speak for all users of Popper, but for Microsoft, this will be a very painful breaking change since large applications like Teams will need to to completely reevaluate all the offsets in the app.

[atomiks]

Hi, thanks for opening this issue!

The reason for logical crossAxis is the following scenario: #1170. I personally encountered this when making a right-start placed sub-menu that could flip to right-end, and used a small crossAxis offset to align menu items correctly when there was padding.

[atomiks]

That said, although it had 4 thumbs up and personally I think it makes more sense than before, I get the need to retain the old behavior for migration purposes.

You can wrap offset with logic that "undoes" the inversion and it's quite small:

import {offset as _offset} from '@floating-ui/dom';

export const offset = (value) => ({
  name: 'offset',
  options: value,
  fn(args) {
    const {placement} = args;
    if (typeof value === 'number') {
      return _offset(value).fn(args);
    }
    return _offset({
      mainAxis: value.mainAxis,
      crossAxis: placement.endsWith('end') ? value.crossAxis * -1 : value.crossAxis,
    }).fn(args);
  }
});

Note: haven't tested it and I think there might need to be an RTL check there but that's the general idea.

[ling1726]

Do the x,y coordinates change based off alignment ? if it's just that offset that I would still consider that as inconsistent. Since the axis is still fixed for the rest of the coordinate system, and it's only the offset axis changing based on alignment

It also adds 'something else to learn' about the library, and honestly I think that supporting this might be too 'high level' for floating UI. I can accept that RTL should be supported since that affects the entire platform and not just a specific use case of floating elements.

Do you think that it's possible to make this new behaviour or the old behaviour optional in the offset middleware ? I'm thinking about something like a logicalAxis boolean option or something like that

[atomiks]

It also adds 'something else to learn' about the library

I think it's that way if you're coming from Popper but for someone new I think it works more intuitively. This is the example I'm talking about — the flipped alignment works correctly with a static crossAxis: -10 value (the submenu is aligned to the button in the middle of the list on the left):

Screen.Recording.2022-03-02.at.11.59.45.pm.mov

---

A logical: boolean option is possible though.

[ling1726]

I wonder why the flip middleware couldn't apply the axis inversion in this case ? since it seems that the problem that is being solved is related to that. I still firmly believe that x, y should be that and any transforms should not be from the actual offset middleware but as a result of other requirements like flipping

I think a logical boolean would be nice to give the option of absolute vs logical axes. Looking at the code it does seem that there is only one check were this axis inversion is applied ?

[atomiks]

The thing is offset didn't do raw x/y transforms before. If the placement is top and mainAxis: 10, y = y - 10, but on the bottom y = y + 10. For left/right it is x = x - 10 and x = x + 10.

Yeah the option would be easy to add. But given it didn't do raw inversions before I'm not sure how much it makes sense 🤔 flip is not concerned with changing any offsets, it should remain agnostic and only focus on one thing

[ling1726]

flip is not concerned with changing any offsets, it should remain agnostic and only focus on one thing

I guess my POV is that the problem that is being solved here is mainly for flip (correct me if I'm wrong) in that case it doesn't make sense for offset to really know about what the axes actually are other than apply offsets as specified. RTL would be the exception I'm happy to make since it applies to the platform.

With middlewareData shouldn't it be quite easy for flip to take into account the data from the offset middleware and 'undo' and modify those offsets accordingly ?

[ling1726]

I ended up copying the offset modifier and removing the ~4 lines of code that does this behaviour, it's easier than working around by 'undoing' the offset and results in less code for the bundle

floating-ui/packages/core/src/middleware/offset.ts

Lines 38 to 40 in a8de472

	if (alignment === 'end') {
	crossAxisMulti = -1;
	}

Still happy to continue the discussion on this though

[atomiks]

It's not really just flip, it makes sense for crossAxis to work consistently for each placement without using flip too

[atomiks]

Instead of logical it would make sense to call this invertEnd or something because it's only changing the end alignment. Without an alignment, it works the same as a start alignment, and this solves the issue with calling it "logical"

[ling1726]

it makes sense for crossAxis to work consistently for each placement without using flip too

IMO consistently would mean that the coordinate system did not change because of how the floating element is aligned. This decision seems more like trying to introduce a convention that positive crossAxis offset will move in the direction of the alignment, which might or might not be easier to understand for everyone. I do think this is convention because in this case why does a center alignment offset like start but not end ?

I guess my main issue (apart from the personal belief that coordinate system should not change) with this is that it is such a change from v2, and migrating this for large codebases can be hard to the impossible, especially from a library POV. Currently the easiest way of bailing out of this behaviour is to copy the offset middleware src and remove the associated lines.

Definitely do appreciate the proposals to bail out back to v2 behaviour in the current API 👍maybe followAlignment or fixedAxis as an option ? since my understanding is that the axis change based on nothing else but alignment

[atomiks]

I def think a boolean that will disable "alignment tracking" is fine. I just have no idea what to call it... crossAxisInvert: boolean or alignmentInversion: boolean maybe?

And yeah, the fact that a center alignment (or no alignment) works like start makes the change a bit ambiguous.

[FezVrasta]

Maybe have a startOffset and endOffset instead?

[atomiks]

alignmentOffset could work, with crossAxis reverting back to how it worked in Popper v2