microsoft / react-native-xaml

A React Native Windows library to use XAML / WinUI controls
MIT License
93 stars 25 forks source link

AppBarButton's aren't correctly re-rendered when the underlying CommandBar isn't rerendered #111

Open dstaley opened 3 years ago

dstaley commented 3 years ago

When React attempts to update the AppBarButton's of a CommandBar when the underlying CommandBar isn't marked for changes, the buttons aren't rerendered, becoming out-of-sync with the React VDOM.

image

(One interesting note from this screenshot: the AppBarButtons are missing children elements after React tries to update them. Normally, they'd have a Root Grid containing a Border and Grid element, the latter of which contains a Viewbox and TextBlock.)

In this example, the <Actions /> component pulls data from a Context provider to determine what AppBarButtons to render. Since CommandBar isn't listening to that context, it doesn't need to rerender on context changes, but <Actions /> does.

import React, {useContext} from 'react';
import {
  CommandBar as XAMLCommandBar,
  CommandBarDefaultLabelPosition,
  VerticalAlignment,
  StackPanel,
  Orientation,
  Button,
  FontIcon,
  TextBlock,
  TextLineBounds,
  AppBarButton,
} from 'react-native-xaml';
import {RouterContext} from './router';
import {ToolbarContext} from './Toolbar';

const BackButton = () => {
  const {canGoBack, goBack} = useContext(RouterContext);
  return (
    <Button
      resources={{
        ButtonBackground: 'Transparent',
        ButtonBackgroundDisabled: 'Transparent',
        ButtonBorderBrushDisabled: 'Transparent',
      }}
      borderBrush="transparent"
      isEnabled={canGoBack}
      onClick={goBack}>
      <FontIcon
        fontFamily="Segoe Fluent Icons"
        glyph="&#xE72B;"
        fontSize={16}
      />
    </Button>
  );
};

const Title = () => {
  const {title} = useContext(ToolbarContext);
  return (
    <TextBlock
      verticalAlignment={VerticalAlignment.Center}
      text={title}
      fontSize={20}
      fontWeight={700}
      textLineBounds={TextLineBounds.Tight}
    />
  );
};

const Actions = () => {
  const {actions} = useContext(ToolbarContext);
  return (
    <>
      {actions.map(action => (
        <AppBarButton
          key={action.title}
          label={action.title}
          onClick={action.onClick}
          isEnabled={!action.disabled}>
          <FontIcon fontFamily="Segoe Fluent Icons" glyph={action.icon} />
        </AppBarButton>
      ))}
    </>
  );
};

export const CommandBar = () => {
  return (
    <XAMLCommandBar
      isOpen={false}
      defaultLabelPosition={CommandBarDefaultLabelPosition.Right}
      verticalContentAlignment={VerticalAlignment.Center}>
      <StackPanel
        orientation={Orientation.Horizontal}
        verticalAlignment={VerticalAlignment.Center}>
        <BackButton />
        <Title />
      </StackPanel>
      <Actions />
    </XAMLCommandBar>
  );
};
asklar commented 3 years ago

the logic in removeChild / replaceChild isn't fully fleshed out yet so that might be why. Are you able to set a breakpoint in those methods and see if they're getting called? they won't do anything interesting right now but once the logic is added, it should work :)

I am planning on making some changes to the codegen so that the logic for these methods can be auto-generated from metadata as well, that's tracked by #53

dstaley commented 3 years ago

So it looks like ReplaceChildAt isn't being called. I'm getting a few calls to RemoveChildAt that aren't being handled, and look to be called with the CommandBar as the parent, and an index > 0. I know I promised not to write C++, but I did at least take a look at adding the logic for CommandBar. I didn't see a method that would allow me to remove a specific child by index though, but it's totally possible that we'd need to interact with the Control property, but I'm not exactly sure how to do that bit.