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

Feature Request: Multiplier value for FxLayoutGap #907

Closed
CharlieGreenman opened this issue Dec 12, 2018 · 13 comments · Fixed by #1383
Closed

Feature Request: Multiplier value for FxLayoutGap #907

CharlieGreenman opened this issue Dec 12, 2018 · 13 comments · Fixed by #1383
Assignees
Labels
discussion Further discussion with the team is needed before proceeding feature has pr A PR has been created to address this issue in-progress P5 Low-priority issue that needs consideration

Comments

@CharlieGreenman
Copy link

CharlieGreenman commented Dec 12, 2018

Feature Request

What is the desired behavior?

FxLayoutGap should have multiplier for attaining a pixel value.

What is the use-case or motivation for the desired behavior?

Well this is an Angular library. Many people who use Angular use Angular Material. Was just playing around with the idea of fxLayoutGap taking in a multiplier. So for instance, Angular Material Layout is generally in multiples of 8, with the exception of icons which can be 4. So fxLayoutGap="1x" would be 8px. fxLayoutGap="2x" would be 16px

Is there anything else we should know?

Nothing other than this is just an idea. Using flex in a css setting, allows us to set a constant that is there already at build time. It's comforting knowing that if we need to change the value across the board, we can do that for our entire app. Right now just throwing out having a default value based on material. However, having a multiplier based on one's style sheet is just a cool idea. Having a multiplier for fxLayoutGap that is stamped with the performance stamp of approval, would be fantastic. Thank you!

// Gutter variables, for padding + margin
// function to take in multiplier(8), which must emit one of the values within fx-space-amounts
@function fx-space-multiplier($n) {
  $fx-space-amounts: (
    0,
    4,
    8,
    16,
    24,
    32,
    40,
    48,
    56,
    64
  );
  $fx-space-multiplier: 8;

  @if (index($fx-space-amounts, ($n * $fx-space-multiplier))) {
    @return #{$n * $fx-space-multiplier}px;
  } @else {
    @error 'Must contain one of the following numbers: #{$fx-space-amounts}.';
  }
}

^ something equivalent to above, but for fxLayoutGap

@ThomasBurleson
Copy link
Contributor

ThomasBurleson commented Dec 13, 2018

@CharlieGreenman - @CaerusKaru and I really like this idea...

@CaerusKaru
Copy link
Member

@CharlieGreenman I think this is an EXCELLENT idea. Somehow it seems like a feature that was here all along...

The only question is on design, otherwise we could get this in in no time. So... should this just be for fxLayoutGap, or should this be a library-wide syntax? Should it be configurable, and if so, where?

I'm leaving these questions up to you Charlie, let me know what you think!

@CaerusKaru CaerusKaru self-assigned this Dec 13, 2018
@CaerusKaru CaerusKaru added feature P5 Low-priority issue that needs consideration discussion Further discussion with the team is needed before proceeding labels Dec 13, 2018
@CaerusKaru CaerusKaru added this to the Backlog milestone Dec 13, 2018
@CharlieGreenman
Copy link
Author

CharlieGreenman commented Dec 13, 2018

@CaerusKaru that is a very good question, and it is very kind of you to leave the question up to me.

... should this just be for fxLayoutGap, or should this be a library-wide syntax?

Good point, makes sense for it to be a library-wide syntax for any spacing related apis. The following are the api's that this multiplier should apply to:

  1. fxLayoutGap
  2. fxFlexOffset

They are the only apis that deal with % | px | vw | vh only.
fxFlex is debatable. A multiplier value seems to go against it's spontaneity.

Should it be configurable, and if so, where?

That is also a very good question. Similar to how we currently offer a provider/token for breakpoints, is how we should approach this one.

Something like offering:

import { MULTIPLIER_VALUE } from '@angular/flex-layout';

export const MultiplierValueProvider = { 
  provide: MULTIPLIER_VALUE,
  useValue: 6
};


@NgModule({
  providers: [
  MultiplierValueProvider,  // Supports override of default multiplier value of 8
 // BreakPointsProvider,     // Supports developer overrides of list of known breakpoints
 // BreakPointRegistry,      // Registry of known/used BreakPoint(s)
 // MatchMedia,              // Low-level service to publish observables w/ window.matchMedia()
 // MediaMonitor,            // MediaQuery monitor service observes all known breakpoints
 // ObservableMediaProvider  // easy subscription injectable `media$` matchMedia observable
  ]
})

That would be ^ along the lines of what I am thinking of. It would throw if developer supplies a value other than whole numbers, or 0.5, to ensure layouts across app are inconsistent. Thank you, and by all means, would be more than happy to discuss more.

@CaerusKaru
Copy link
Member

Sounds good on making it only apply to those two directives, and probably also gdGap.

I was thinking maybe we would add this to LayoutConfigOptions. And that maybe we wouldn't throw an error for non-integer values, but those would obviously render weirdly when the values get applied.

Do you want to put together a PR for this, or would you rather we implement it? If you want the commit credit, I think you deserve it!

@CharlieGreenman
Copy link
Author

CharlieGreenman commented Dec 13, 2018

@CaerusKaru I will work on it from my end. If it takes longer than by this Sunday, I will let you know. Thank you.

@CharlieGreenman
Copy link
Author

Hey won't be able to get around to this, this weekend. If we have the option to keep this around for a week, or two, I can knock this out.

@CaerusKaru
Copy link
Member

It’s all yours, no rush!

@CharlieGreenman
Copy link
Author

Perfect 🙂

@CharlieGreenman
Copy link
Author

CharlieGreenman commented Dec 30, 2018

@CaerusKaru still working on it. That being said, I like this as an open source project, more so than the rest. Obviously any code base is difficult for the first time. If there is a way we can be in direct contact so that I can ask for pointers here and there, that would be great. In return for time given, I would contribute on this and others in the near future. Thank you.

@ThomasBurleson
Copy link
Contributor

Hi @CharlieGreenman - let me create a Slack channel for team contributors. ;-).

@ThomasBurleson
Copy link
Contributor

@CharlieGreenman - please send me your email to [email protected]

@CaerusKaru
Copy link
Member

CaerusKaru commented Jan 3, 2022

Not sure if anyone is still actively waiting on this at this point, but I've roughed up an implementation in #1383, which should land in the next release.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion Further discussion with the team is needed before proceeding feature has pr A PR has been created to address this issue in-progress P5 Low-priority issue that needs consideration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants