Skip to content

Conversation

@siddharthkp
Copy link
Contributor

@siddharthkp siddharthkp commented Jan 7, 2020

Built on top of: #3256

First component, lots of concepts introduced here. See changes for comments.

@siddharthkp siddharthkp changed the base branch from master to components-package January 7, 2020 10:57
@lbogdan lbogdan temporarily deployed to pr3269 January 7, 2020 11:04 Inactive
@lbogdan
Copy link
Contributor

lbogdan commented Jan 7, 2020

Build for latest commit f217554 is at https://pr3269.build.csb.dev/s/new.


configure(loadStories, module);
// automatically import all files ending in *.stories.js
configure(require.context('../src', true, /\.stories\.tsx$/), module);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed the declaration so that we can use their new format, see collapsible/stories.tsx

@lbogdan lbogdan temporarily deployed to pr3269 January 7, 2020 11:47 Inactive
alignItems: 'center',
height: '32px',
paddingX: 3,
borderBottom: '1px solid',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SaraVieira This collapsible has too much information about the sidebar, should we just call it a SidebarSection instead, or should we break it into 2 parts where one non-styled part lives in components and the other extends in app/sidebar/?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, when adding the animation I think it would make sense to call it SidebarSection

@lbogdan lbogdan temporarily deployed to pr3269 January 7, 2020 13:07 Inactive
@lbogdan lbogdan temporarily deployed to pr3269 January 7, 2020 13:41 Inactive
@lbogdan lbogdan temporarily deployed to pr3269 January 7, 2020 14:04 Inactive
@SaraVieira SaraVieira changed the base branch from components-package to master January 7, 2020 14:46
@SaraVieira
Copy link
Contributor

Missing animation but all the rest looks good so merging because it creates a lot of foundations

@SaraVieira SaraVieira marked this pull request as ready for review January 7, 2020 14:48
@SaraVieira SaraVieira merged commit b8085fc into master Jan 7, 2020
@SaraVieira SaraVieira deleted the collapsible branch January 7, 2020 15:10
@siddharthkp siddharthkp mentioned this pull request Jan 7, 2020
step?: number;
value: number;
} & Pick<ComponentProps<typeof Input>, 'style'>;
style?: React.CSSProperties;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@siddharthkp This should be reverted (see @SaraVieira's PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants