Component/ae splash - Githubissues

aeternity / aepp-components

deprecated: aepp-components to be used in all aepps.

ISC License

41 stars 14 forks source link

Component/ae splash #132

Closed ghost closed 5 years ago

ghost commented 6 years ago

task: ae-splash component will serve as a splash placeholder for when the user enters the app or for when things need a bit of emphasis

davidyuk commented 5 years ago

This should be a part of the Base app, is not it? Similar functionality already implemented in https://github.com/aeternity/aepp-identity/blob/develop/src/pages/Intro.vue

sadiqevani commented 5 years ago

@davidyuk

I'll create a general solution that can cover more use-cases than the base app.

sadiqevani commented 5 years ago

@davidyuk

up for review.

sadiqevani commented 5 years ago

@davidyuk

We need to take a decision on this component, I have no specific opinion in the case for this type of component.

In one aspect it's quite a simple component, but because its simple, it can be enhanced in the future to something better. Also, if you have already a use-case in the base-app, then I believe its also slightly useful.

If you had to "code" this kind of layout several times in the base-app, why not simply create a reusable component.

Actions to take for this component:

Close this PR and forget about the component.
- Please provide some kind of counterargument as in why we should remove it.
Merge this PR and continue work as usual.

davidyuk commented 5 years ago

In the Base app, we are using general MobilePage component, the layout is implemented once there. Or I have understood you wrong, so then please provide duplicate layouts in the code of the Base app.

The functionality of AeSplash that is needed for the Base app implemented by this line of styles.

sadiqevani commented 5 years ago

In the Base app, we are using general MobilePage component, the layout is implemented once there. Or I have understood you wrong, so then please provide duplicate layouts in the code of the Base app.

I am speaking in components terms, you already implemented that in the base app, but speaking for other applications, are we going to reinvent the wheel over again?

The functionality of AeSplash that is needed for the Base app implemented by this line of styles.

Yes, but this is aepp-components and this library is going to be used in multiple applications. This library is not only for the base-app.

davidyuk commented 5 years ago

I think app layouts are too specific to extract them to the components package. If somebody will use this component and he needs some enhancing of it, he may override some of its styles, but it is less maintainable because to check the complete list of styles applied to the layout developer also should check sources of this component.

I'm personally trying to keep styles cleaner, flex-direction: column, overflow-x: hidden looks not necessary for layouts in general, so I will avoid usage of this component, or I have to revert this styles in some cases that make code less obvious.

sadiqevani commented 5 years ago

@davidyuk Then for this component, I guess we can discard if you think there's no value to it!.