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

Implement better ordering #138

Open
EvanLovely opened this issue Oct 12, 2017 · 10 comments
Open

Implement better ordering #138

EvanLovely opened this issue Oct 12, 2017 · 10 comments
Assignees

Comments

@EvanLovely
Copy link
Member

If there is a MD doc with this:

---
order: -5
---

It should sort by that and then name. The default of order is 0. This is already implemented in the node version and was implemented in this version here: #137 - so that makes order available in the Pattern Data and ready to implement.

@sghoweri
Copy link
Contributor

@EvanLovely I should be able to tackle this one tomorrow or Saturday.

Off the top of my head I think we'll just need to sort the array of pattern objects in PL first (before anything gets compiled) based off of those specifying an order before handling the rest with the default order of zero. Should be a pretty easy win (and be a huge help with not having to rename files based on reordering OR based on their pattern state)

@sghoweri sghoweri self-assigned this Oct 12, 2017
@bradfrost
Copy link
Member

Big +1 on this. I remember @davatron5000 discussing this way back in the day, so it would be great to roll that into our other conventions so file names can stay consistent and we can lose the 00- prefixes.

@sghoweri
Copy link
Contributor

Just want to confirm a couple things after thinking this through a bit more.

  1. The final order of a pattern (in each top level category like atoms, etc) is going to be based on several potential factors, in order of priority / "specificity":
  • The order key in .md file if it exists (or potentially a different config file like .json or .yml down the road)**
  • A numeric prefix if it exists (which I'm thinking I might try to convert to an order value before .md order values are sorted with the highest priority) -- in either case, I want to make sure this doesn't end up breaking even if the data gets simplified when a pattern gets processed
  • The default order based on file name (alphanumeric sort)

**if we were to support multiple ways of configuring a pattern, I'd suggest we either:
A. do what fractal does and only worry about a pattern's pattern.config.xx file if it exists OR

B. if somehow multiple file configs existed (ex. a .yml config value and .md config value, then simply process all of those config types equally and the last one processed wins (.yml in this case)

  1. Let's say we did convert a number prefix on a file into an internal order key. Shouldn't we then strip off the number from the pattern at that point? (meaning, the .html files created would no longer have that order in the path since the order is a complex internal thing being handled, not a literal sort based on file name)

Does this all kinda make sense @EvanLovely @bradfrost?

@sghoweri
Copy link
Contributor

sghoweri commented Oct 15, 2017

Ok, @EvanLovely @bradfrost good news and bad news!

Good news: I think I got this working in the main nav as a POC (even though it took me most of the day today to deconstruct and reconstruct what PL is doing under the hood ) 🍺 🍻 😪

Bad news: looks there might be a bug (existing bug?) relating to the navigation / nav data when markdown files are added for patternSubTypes that this work seems to have surfaced (ex. if I had a molecules folder and placed a buttons.md file inside to reorder the top level buttons category).

Need to get what I have locally in a stable spot before I can push anything up but in any case, making progress here!

@bradfrost
Copy link
Member

bradfrost commented Oct 15, 2017

Thanks for rolling up your sleeves with this, @sghoweri! I'll avoid getting in the weeds (largely because I'm unsure of what's going on under the hood), but I think the spirit of the task is as follows:

  • The current behavior is directories and patterns are organized alphabetically. So a folder named components would appear before a folder named modules. That should continue to be the case.
  • If one wanted to put a directory or pattern in front of another (i.e. someone like me would want to put /templates in front of /pages or header.mustache in front of footer.mustache), a user would create a markdown file of the same name and declare an order value. The integer value of that key would determine where it shows up in the nav and the view all template.

Do I have that right?

To address some of the things in here. @EvanLovely said:

The default of order is 0.

As a user, if I gave something a value of order: 1, I'd expect that thing to appear first in line, before all the other non-specified items (with implicit value order: 0). It might be a good idea to borrow from tabindex here:

An element with tabindex="0", an invalid value, or no tabindex attribute should be focused after elements with positive tabindex values in the sequential keyboard navigation order.

Is that what you had in mind, @EvanLovely?

@sghoweri said:

The order key in .md file if it exists (or potentially a different config file like .json or .yml down the road)**

With respect to multiple config files, I think that it should be a possibility to support .json or .yml config files, but having those multiple configs exist simultaneously should be discouraged (or disallowed?). In any case, for order it seems to make sense to have the last-defined order property be the one that sticks.

Sorry I can't be of any help on the implementation stuff, but if you want to run some ideas by an idiot user, just ping me!

@EvanLovely
Copy link
Member Author

EvanLovely commented Oct 16, 2017

Thanks @sghoweri - you rock! 🎉

I choose the default value of order as 0 since that is what happens for the flex box order property, but I like what @bradfrost suggested:

As a user, if I gave something a value of order: 1, I'd expect that thing to appear first in line, before all the other non-specified items (with implicit value order: 0). It might be a good idea to borrow from tabindex here:

An element with tabindex="0", an invalid value, or no tabindex attribute should be focused after elements with positive tabindex values in the sequential keyboard navigation order.

So it should go:

  1. All items with order set, ascending from 1 up. Having 0 places in next category and negative numbers are not allowed. Thoughts on allowing 1.5?
  2. All items without order.

Everything is finally settled alphabetically (which would also put 1 before 2 and 11 - and should not need 01 to go before 11).

Also, I brought this up in The Spec to formalize using order: pattern-lab/the-spec#34

@sghoweri
Copy link
Contributor

@EvanLovely so wait, negative numbers wouldn't be allowed? Why wouldn't we do something like this? https://codepen.io/sghoweri/pen/YrggVp

IMO we should only allow integers to keep things relatively simple (ya gotta pick -- either make something heavier to drop it's natural location down or lighten it up to raise it)

@sghoweri
Copy link
Contributor

Also, I'm gonna shoot for getting at least half my code for this posted this weekend (crazy work week) - thinking 1 PR for globally setting order values for all Pattern Object data that needs it + a separate one for the actual reordering itself.

I found that trying to make changes to the order PL naturally wants to use is immensely easier in the Pattern Exports themselves than to try to do it before the Pattern rules get run through...

sghoweri pushed a commit that referenced this issue Oct 22, 2017
…new `order` property (using the primary nav at the moment to validate everything is working as expected). Major part of the underlying work behind #138
@sghoweri
Copy link
Contributor

Ugh. Still working through some strange behavior on this one (on the feature/reorder-patterns branch I just pushed up).

Just focusing on getting the dropdown navigation / nested nav items sorted using this atm but even that is proving to being harder than expected. I've got this working partially (at least w/ the nav items that have order values explicitly set) but now that strangely throwing off the other nav items that don't have this value set / overwritten... HMMMM

sghoweri pushed a commit that referenced this issue Oct 30, 2017
… patternTypes reordered + non-nested patterns working! Addresses #138
@lquessenberry
Copy link

I am using Pattern Lab Twig. Is there a functionality within it to order your patterns by config file?

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

No branches or pull requests

4 participants