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

TASK :: Revise Drupal + Pattern Lab Directory Structure #831

Merged
merged 7 commits into from
Jun 7, 2020

Conversation

jryanconklin
Copy link
Contributor

Pull Request

Closes Issue #827

Issue Description

Revise directory structure and docs for easier understanding.

Summary of Changes

Revise the directory structure and update Drupal documentation. This PR:

  • Update apps/drupal => apps/drupal-defaultto match thing-designsystem naming
  • Update Drupal Theme Path and rename apps/drupal-default/particle_theme
  • Update Drupal Module and rename apps/drupal-default/particle_helper
  • Update apps/node-pl => apps/node-pl-defaultto match thing-designsystem naming

@mike-potter
Copy link
Member

I feel like node-pl-default should just be pl-default. Don't see the need for the node distinction anymore. Also not generally a fan of -default everywhere. You could easily have a convention of thing-designsystem where if designsystem was not specified it would use default as the default.

Don't hugely care as long as the URLs dont end up being even longer for getting to PL from Drupal /dist/apps/node-pl-default/pl or something.

Finally, in actual Drupal sites using Octane, the theme will be in /project/themes/THEMENAME and the PL design system (Particle) will be in /project/design/particle (until Particle moves to being an actual node dependency, at which point the "source" patterns would be in project/design/DESIGNNAME. So just want to ensure Particle config will still allow that kind of directory structure.

@illepic
Copy link
Contributor

illepic commented Jun 4, 2020

If we're in here making these changes (finally!), let's drop "node" off the app name.

@mike-potter, we're looking for a quick naming convention to tie an "app" to a "design system", since a drupal theme and a pattern lab (and, eventually, a storybook) consume and represent exactly one design system. We're just looking for a quick glance way of knowing "what is this app showing off?" to make the eventual particle 11 generators work.

Those generators will effectively ask us these questions in this order:

  1. What do you want to name your design system (default: "default")?
  2. Where should it be placed (default: source/default)?
  3. Which apps (drupal|pl) would you like to consume this design system (default: both selected)?
  4. Where should those apps live (default: apps/)?

@illepic illepic closed this Jun 4, 2020
@illepic illepic reopened this Jun 4, 2020
@illepic
Copy link
Contributor

illepic commented Jun 4, 2020

Whoops, hit tab and accidentally closed! My bad!

@jryanconklin
Copy link
Contributor Author

jryanconklin commented Jun 4, 2020

I feel like node-pl-default should just be pl-default. Don't see the need for the node distinction anymore. Also not generally a fan of -default everywhere. You could easily have a convention of thing-designsystem where if designsystem was not specified it would use default as the default.

Don't hugely care as long as the URLs dont end up being even longer for getting to PL from Drupal /dist/apps/node-pl-default/pl or something.

Finally, in actual Drupal sites using Octane, the theme will be in /project/themes/THEMENAME and the PL design system (Particle) will be in /project/design/particle (until Particle moves to being an actual node dependency, at which point the "source" patterns would be in project/design/DESIGNNAME. So just want to ensure Particle config will still allow that kind of directory structure.

Default isn't really meant to be kept and I actually like it as a pain point to rename your DS from default. But moreover this will clear the way to setting a DS name as config in 11. I think default is fine as a starting point but it can as easily be base (though I question the labor involved to make that change on 10). TLDR: this will be useful for generators and just basic cognitive understanding of what apps are doing.

I don't think the docs should count on Octane project structure. That tooling has everything it needs to know where to put Particle. Whereas the docs need to cover how to use this elsewhere.

@jryanconklin
Copy link
Contributor Author

@illepic I'll update to remove node from apps/node-pl-default

@jryanconklin
Copy link
Contributor Author

@illepic @mike-potter Updated PR:

  • apps/node-pl-default => apps/pl-default
  • Config for app-pl-node => app-pl

@mike-potter
Copy link
Member

OK, that looks good.

I wasn't asking for a change in docs, I just wanted to make sure that the directory structure needed for Octane can be supported. Sounds like the answer is "yes". And I am fine with default being a "pain point" to encourage projects to name their design system and theme correctly.

@bloom so in your questions, Octane would answer:

What do you want to name your design system (default: "default")? octane
Where should it be placed (default: source/default)? project/design/octane
Which apps (drupal|pl) would you like to consume this design system (default: both selected)? both
Where should those apps live (default: apps/)?
this question needs to be asked for each app
drupal: project/themes/octane
pl: project/apps/pl (or something like that...we don't currently use PL outside of the DESIGN_PATH but supporting this same app concept would be useful)

@illepic
Copy link
Contributor

illepic commented Jun 7, 2020

This no longer runs locally via npm start (even after an npm run setup). I'm looking into why now.

@jryanconklin
Copy link
Contributor Author

Boo. Okay. I can take a look, too. Did you pull without doing a full reset? I have been nuking everything between steps and verifying with ‘npm run ci’

@jryanconklin
Copy link
Contributor Author

@illepic, confirmed on my local machine. Are you deleting node modules and removing old dist?

@illepic
Copy link
Contributor

illepic commented Jun 7, 2020

NOPE. That's my machine due to some port mappings I had to do recently for Docksal. Carry on! I'll get this figured out.

@illepic illepic merged commit 57a64ec into master Jun 7, 2020
@illepic illepic deleted the task/update-drupal-app-structure branch June 7, 2020 22:13
madhaze added a commit that referenced this pull request Jun 8, 2020
* master:
  Update generator demo yml files to have Demo info above content. (#825)
  SVG with Tailwind. (#829)
  TASK :: Revise Drupal + Pattern Lab Directory Structure (#831)
  Issue/827 10.8.0 drupal docs (#828)

# Conflicts:
#	apps/pl-default/pattern-lab/_patterns/02-molecules-demo/accordion/accordions.twig
#	apps/pl-default/pattern-lab/_patterns/02-molecules-demo/accordion/accordions.yml
#	apps/pl-default/pattern-lab/_patterns/02-molecules-demo/accordion/index.js
madhaze added a commit that referenced this pull request Jun 8, 2020
* master:
  Update generator demo yml files to have Demo info above content. (#825)
  SVG with Tailwind. (#829)
  TASK :: Revise Drupal + Pattern Lab Directory Structure (#831)
  Issue/827 10.8.0 drupal docs (#828)

# Conflicts:
#	apps/pl-default/pattern-lab/_patterns/02-molecules-demo/tabs/index.js
#	apps/pl-default/pattern-lab/_patterns/02-molecules-demo/tabs/tabss.twig
#	apps/pl-default/pattern-lab/_patterns/02-molecules-demo/tabs/tabss.yml
madhaze added a commit that referenced this pull request Jun 8, 2020
* master:
  Update all deps. (#835)
  Update generator demo yml files to have Demo info above content. (#825)
  SVG with Tailwind. (#829)
  TASK :: Revise Drupal + Pattern Lab Directory Structure (#831)
  Issue/827 10.8.0 drupal docs (#828)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants