-
Notifications
You must be signed in to change notification settings - Fork 14
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
docs: Metrics storybook design #1462
Conversation
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.
Well done, @solid-maxim @solid-tetianamovlian! I've added a couple of suggestions below - please let me know what you think!
metrics/web/docs/features/storybook/01_metrics_storybook_design.md
Outdated
Show resolved
Hide resolved
metrics/web/docs/features/storybook/01_metrics_storybook_design.md
Outdated
Show resolved
Hide resolved
metrics/web/docs/features/storybook/01_metrics_storybook_design.md
Outdated
Show resolved
Hide resolved
metrics/web/docs/features/storybook/01_metrics_storybook_design.md
Outdated
Show resolved
Hide resolved
metrics/web/docs/features/storybook/01_metrics_storybook_design.md
Outdated
Show resolved
Hide resolved
metrics/web/docs/features/storybook/01_metrics_storybook_design.md
Outdated
Show resolved
Hide resolved
metrics/web/docs/features/storybook/01_metrics_storybook_design.md
Outdated
Show resolved
Hide resolved
metrics/web/docs/features/storybook/diagrams/metrics_storybook_class_diagram.puml
Outdated
Show resolved
Hide resolved
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.
Well done, @solid-tetianamovlian & @solid-maxim! I've added few suggestions here - please take a look.
metrics/web/docs/features/storybook/diagrams/metrics_widgets_structure_diagram.puml
Outdated
Show resolved
Hide resolved
|
||
In general, it consists of three main components: | ||
|
||
- **Story** - a class, that groups together a list of chapters. |
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.
:readable:
Would it be great to explain more low-level components first, and then go to more top-level one? I mean it would be great to have the following order:
- Chapter
- Chapter Controls
- Story
Also, would it be great to explain more widely what does Story
, Chapter
, and Chapter Controls
mean?
metrics/web/docs/features/storybook/01_metrics_storybook_design.md
Outdated
Show resolved
Hide resolved
metrics/web/docs/features/storybook/01_metrics_storybook_design.md
Outdated
Show resolved
Hide resolved
metrics/web/docs/features/storybook/01_metrics_storybook_design.md
Outdated
Show resolved
Hide resolved
metrics/web/docs/features/storybook/diagrams/metrics_storybook_stories_class_diagram.puml
Outdated
Show resolved
Hide resolved
} | ||
|
||
Story -right-> Chapter : uses | ||
Chapter -right-> ChapterControls : creates |
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.
:readable:
Would it be great to explain the way of creation of the ChapterControls
?
abstract class Chapter { | ||
+ name : String | ||
+ controls : ChapterControls | ||
|
||
+ build() : Widget | ||
} |
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 do you think about splitting the data model and the view into different classes? I believe it would be great to have a widget and a Chapter
model for each Metrics widget. Also, I believe it would be cleaner and more structured if we'll have view models for each Metrics widget and for each widget representing a ChapterControl
so that we'll have a ChangeNotifier
containing a list of Stories
and a selected Chapter
. Also, we'll have a ChangeNotifier
that will hold and control the ChapterControls
for the selected Chapter
and provide a view model used to build it.
So with that end-users will use the sidebar to choose an interested widget, change its appearance through a list of inputs in the editing panel, change the theme using the toggle theme button, and view an actual result in the preview. | ||
|
||
### Program | ||
> Detailed solution description to class/method level. |
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.
:readable:
I believe it would be great to be more consistent in this section. I mean it would be nice to explain firstly the main classes and interfaces of the Storybook
, why do we need them, what is the responsibility of each of them. Then it would be great to explain the way of using these interfaces, provide an example of the implementor. After these things are covered, we should explain the mechanism of these interface's interactions and the way users would interact with the application (UI components, etc.). Eventually, we should provide a section explaining the relationships between all these classes/interfaces, a sequence diagram explaining how the application would work overall.
Close this PR for now as we want to improve the storybook design document. |
Fixes #1454
Checklist