[go: up one dir, main page]

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

[Icon Request] NGRX Icons #1069

Open
2 tasks done
Kaffiend opened this issue Jul 15, 2017 · 13 comments
Open
2 tasks done

[Icon Request] NGRX Icons #1069

Kaffiend opened this issue Jul 15, 2017 · 13 comments

Comments

@Kaffiend
Copy link
Kaffiend commented Jul 15, 2017
  • I'm sure this issue is not a duplicate?
  • I want to create an icon request(if not, remove these lines below):
    - Type: extension
    - Icon Name: ngrx
    - Sample original Icon: url
    - Extensions: .state .*state, .actions, .reducer, .*reducer, .effects

could conflict with redux icons, if any. only applies to angular icons.

@Kaffiend Kaffiend changed the title [Request] NGRX Icons [Icon Request] NGRX Icons Jul 15, 2017
@jens1o jens1o added this to the Backlog milestone Jul 15, 2017
@jens1o
Copy link
Member
jens1o commented Jul 15, 2017

I'm afraid, but somewhat like .*state is not supported by vscode, so we cannot provide that, yet.

@JimiC
Copy link
Member
JimiC commented Jul 15, 2017

@robertohuertasm needs to have a look at this. I suspect it's something similar to the smart-component thing.

@Kaffiend
Copy link
Author

.state.ts would handle the majority, as well as .reducer.ts but some people use rootReducer.ts and rootState.ts etc. and some even use the index.ts in a reducer folder as the root state. Obviously nothing to be done there.

@robertohuertasm
Copy link
Member

This has been something that have crossed my mind several times but, honestly, I didn't find it so valuable as to bring it into the table.

Although these extensions are commonly used in ngrx most of them are also used in all redux-based implementations so, in case we thought of implementing this, we shouldn't approach it as an Angular iconset extension. Furthermore, it should be optional. Maybe we should add some detection functionality looking for ngrx, redux, vuex in the package.json.

But, as I said, I have many doubts about the benefit of implementing these icons:

  1. These should be generic icons so it would suppose a huge difficulty finding and/or designing them.
  2. Normally, people organize actions, reducers, state and effects in a specific folder. See ngrx example and redux example
  3. I guess the extension itself along with the folder structure is enough to identify the purpose of the files so that's why I don't really see a strong point in favor of implementing this.

As a final reminder to @Kaffiend, I guess you already know that we provide the option to customize the file associations.

@JimiC, @jens1o, do you have an opinion on this matter?

@robertohuertasm
Copy link
Member
robertohuertasm commented Jul 15, 2017

I forgot to comment that when #933 is delivered you will be able to add your custom icons into the same repo or even a shared location making it easy to share your custom icons with your team.

@JimiC
Copy link
Member
JimiC commented Jul 15, 2017

Technically ngrx detection can be done via PAD. It's the art part that's troubling me. And once we find them we may have to create also folder icons with them (maybe PAD can be expanded to control folder icons too).

@jens1o
Copy link
Member
jens1o commented Jul 15, 2017

I think we can do that... But it's a huge job to get this done and this would introduce a lot of testing, while most of the users don't actually want this... We have around 2.5 million downloads and only one request. Okay, when we get this done, and I'm really looking forward to see this working, I'm absolutely fine. I think this would be a big surprise and would add another value to this project. :)

@Kaffiend
Copy link
Author
Kaffiend commented Jul 15, 2017

Here is an image
28226848-9903db36-68a5-11e7-9056-f37d8a77067e
and this is the official ngrx image, with the angular shield and rx icon. different iterations of color could be used to differentiate?

edit: @robertohuertasm regarding the folder structure with the ngrx 4.x release coming that may change with the async feature state and actions residing within each module or component as the example app is just one organization pattern. This release adds a lot more flexibility. I do not agree that redux should be included in the detection as ngrx is not redux, and redux though similar structure and behavior could have entirely different icons but im not against more generic icons to cover both bases either. ngrx also has an effects module that redux does not...that im aware of.

@JimiC
Copy link
Member
JimiC commented Jul 16, 2017

@Kaffiend Technically we can support ngrx and redux project's detection. It's the art part that will take time. Currently, the team is busy with RL staff and there is not much free time to dedicate to the project. We will iterate on this again by Fall.

@robertohuertasm
Copy link
Member
robertohuertasm commented Jul 16, 2017

@Kaffiend, obviously ngrx is not redux but it's clearly based upon it and thus they share some of the suffixes like reducers, actions... I don't see the point on why this should only be reduced to ngrx. If we're going to do this I would aim also for redux, ng-redux and vuex (although the latest diverges in some of the terms like actions).

I use ngrx on a daily basis so it's not that I'm against this because of ignorance of the library. It's just that I'm not sure that this will add any value aside from more auto-detection prompts and additional restarts once you change between projects. As a user, this is a thing I'll try to avoid at all cost. That being said, I'm fully aware that PAD is completely optional so changes can be done manually but the point would be that if I have several projects with different state management libraries like redux and ngrx which are based on the same pattern I wouldn't like to be even manually changing the icons.

Anyway, I agree with @JimiC in postponing this discussion after summer. We're all really busy aside from the project and this is not a priority issue.

@KingDarBoja
Copy link
Member
KingDarBoja commented Apr 13, 2019

Anything I can help on this issue?

Update: I made some NGRX icons based on this request on material-icons and using this color palette as guideline.

You can check them at my branch here. Hopefully, we will make this work with PAD but I should better wait for V9 to be released in order to do these changes.

@JimiC @robertohuertasm Thoughts?

@georgeat8
Copy link

Any news on this, since Material-Icons provided an example/solution for this (as per @KingDarBoja comment)?

@robertohuertasm
Copy link
Member

Sorry @georgeat8 the project is in low maintenance mode and this would still required to code the PAD functionality. Not sure when we'll have bandwidth for this.

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

6 participants