Skip to content

Persona: mergeStyles part 3, split sub-components#4496

Merged
jordandrako merged 5 commits intomicrosoft:masterfrom
jordandrako:mergeStyles/persona-part3
Apr 9, 2018
Merged

Persona: mergeStyles part 3, split sub-components#4496
jordandrako merged 5 commits intomicrosoft:masterfrom
jordandrako:mergeStyles/persona-part3

Conversation

@jordandrako
Copy link
Copy Markdown
Contributor

@jordandrako jordandrako commented Apr 9, 2018

Pull request checklist

  • Include a change request file using $ npm run change

Description of changes

  • Split PersonaCoin.tsx into PersonCoin.tsx and PersonaCoin.base.tsx
  • Split PersonaPresence.tsx into PersonPresence.tsx and PersonaPresence.base.tsx
  • Move PersonaCoin and PersonaPresence into subfolders.

@jordandrako
Copy link
Copy Markdown
Contributor Author

jordandrako commented Apr 9, 2018

@dzearing @oengusmacinog Should sub-components like PersonaCoin and PersonPresence live in their own folders like the Button folder structure to cut down on clutter?

Persona >
|  examples >
|  PersonaCoin >
|  |  index.ts
|  |  PersonaCoin.base.tsx
|  |  PersonaCoin.styles.ts
|  |  PersonaCoin.tsx
|  PersonaPresence >
|  |  index.ts
|  |  PersonaPresence.base.tsx
|  |  PersonaPresence.styles.ts
|  |  PersonaPresence.tsx
|  index.ts
|  Persona.base.tsx
|  Persona.styles.ts
|  Persona.tsx
|  PersonaPage.tsx

@jordandrako jordandrako changed the title Merge styles/persona part3 Persona: mergeStyles part 3, split sub-components Apr 9, 2018
export * from './Persona.base';
export * from './Persona.types';
export * from './PersonaCoin';
export * from './PersonaCoin.base';
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

probably want to add PersonaPresence.base too

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Left out for now as Persona.types export PersonaPresence enum so the names conflict. What are your thoughts on this:

export { PersonaPresence as PersonaPresenceIcon } from './PersonaPresence';
export { PersonaPresenceBase as PersonaPresenceIconBase } from './PersonaPresence.base';

Copy link
Copy Markdown
Collaborator

@oengusmacinog-zz oengusmacinog-zz left a comment

Choose a reason for hiding this comment

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

Just the one comment, I would go ahead and add presence base to index but otherwise looks good mate!

@oengusmacinog-zz
Copy link
Copy Markdown
Collaborator

For File structure, I think what you have is good. Might start getting confusing as we scale, but essentially nested components would be intended to be subcomponents of the folder they are nested in, even though each sub-component can be used individually if desired. Still seems cleaner to structure this way, just should document some standards of when to nest and export a component

@jordandrako jordandrako merged commit b8cde7c into microsoft:master Apr 9, 2018
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants