-
Notifications
You must be signed in to change notification settings - Fork 38
Resolve GitHub issue #64: "Maintain a list of frontend grid sizes and export them" #71
Conversation
src/InteractiveConstants.ts
Outdated
*/ | ||
public static get gridLayoutSizes(): IGridLayout[] { | ||
return {...InteractiveConstants._gridLayoutSizes}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use Object.freeze for the same effect, which returns a ReadOnlyArray
for compile-time enforcement for immutability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohhh, right, why didn't I think of that ...
I'll improve that once I'm home 😬
src/InteractiveConstants.ts
Outdated
* Offers constant information values to use in an application. | ||
*/ | ||
export class InteractiveConstants { | ||
private static readonly _gridLayoutSizes: IGridLayout[] = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be a class?
Could it be a module instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checkout InteractiveError
in errors.ts for an example of a module.
-> "Maintain a list of frontend grid sizes and export them"
…y readonly (also sorted modules in index.ts by the alphabet)
/** | ||
* Offers constant information values to use in an application. | ||
*/ | ||
export module InteractiveConstants { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for wrapping this inside of a module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where else could you place such information? 🤔 As a static method in Scenes?
Also that way other constants for stuff could be grouped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file is already called constants
why not just export it directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured that if there's more constants stuff to be exported in the future, you'd wanna wrap it anyways 😊
Makes it a bit cleaner in my opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't really use modules to wrap things up
anywhere in TS, namespaces maybe, but not modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, feel free to change it then!
I won't be able to for the next two weeks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will merge and change it after.
This is an attempt to resolve issue #64 "Maintain a list of frontend grid sizes and export them".
I'm not sure if the method of how this is implemented is the best way, but I think it's okay.
I made it so that the information exported cannot be overridden.