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

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

Open
dstaley opened this issue Jul 17, 2021 · 2 comments
Labels
bug Something isn't working

Comments

@dstaley
Copy link
Contributor

dstaley commented Jul 17, 2021

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
Copy link
Member

asklar commented Jul 18, 2021

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
Copy link
Contributor Author

dstaley commented Jul 18, 2021

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.

@asklar asklar added the bug Something isn't working label Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants