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

UI Guidelines&Front-end refactoring #367

Open
2 tasks
Sven-TBD opened this issue Jul 3, 2023 · 23 comments
Open
2 tasks

UI Guidelines&Front-end refactoring #367

Sven-TBD opened this issue Jul 3, 2023 · 23 comments
Assignees
Labels
enhancement New feature or request refactor UserStory a brief, plain-language explanation of a feature or functionality

Comments

@Sven-TBD
Copy link
Contributor

Sven-TBD commented Jul 3, 2023

Multiple implementations of browser-style codes exist currently. In order to maintain clean frontend code and consistent page styles, a unified UI guideline is needed to specify the interaction logic and layout for each component.

  • UI design

  • Front-end modification

@Sven-TBD Sven-TBD added the enhancement New feature or request label Jul 3, 2023
@Kirl70
Copy link

Kirl70 commented Jul 4, 2023

Since the previous design drafts were based on the existing online pages, some inconsistent styles were also reflected in the design drafts to meet the rapid iteration requirements. Now, if we need to establish design specifications, it might be better to redesign the browser with a new style. The time required for adjusting and redesigning according to the specifications is approximately the same because there are quite a few details that need to be standardized if we want to make changes to the existing online pages.

@Keith-CY what are your thoughts? Do we currently have a need to redesign the CKB explorer?

@IronLu233
Copy link

Since the previous design drafts were based on the existing online pages, some inconsistent styles were also reflected in the design drafts to meet the rapid iteration requirements. Now, if we need to establish design specifications, it might be better to redesign the browser with a new style. The time required for adjusting and redesigning according to the specifications is approximately the same because there are quite a few details that need to be standardized if we want to make changes to the existing online pages.

@Keith-CY what are your thoughts? Do we currently have a need to redesign the CKB explorer?

We do not need redesign all explorer, just refine the design system from existing design.

@Keith-CY
Copy link
Member

Keith-CY commented Jul 4, 2023

Since the previous design drafts were based on the existing online pages, some inconsistent styles were also reflected in the design drafts to meet the rapid iteration requirements. Now, if we need to establish design specifications, it might be better to redesign the browser with a new style. The time required for adjusting and redesigning according to the specifications is approximately the same because there are quite a few details that need to be standardized if we want to make changes to the existing online pages.

@Keith-CY what are your thoughts? Do we currently have a need to redesign the CKB explorer?

Redesigning CKB Explorer may take a lot of time, not only because of the design work itself, but also due to the agreement from the foundation.

But we can start the design spec first, without regarding any specific projects.

The design spec can be used to direct projects other than Neuron and CKB Explorer. And once we have a fully polished design spec, we will propose a new design of Neuron and CKB Explorer to the foundation

@Keith-CY
Copy link
Member

Keith-CY commented Jul 4, 2023

Since the previous design drafts were based on the existing online pages, some inconsistent styles were also reflected in the design drafts to meet the rapid iteration requirements. Now, if we need to establish design specifications, it might be better to redesign the browser with a new style. The time required for adjusting and redesigning according to the specifications is approximately the same because there are quite a few details that need to be standardized if we want to make changes to the existing online pages.
@Keith-CY what are your thoughts? Do we currently have a need to redesign the CKB explorer?

We do not need redesign all explorer, just refine the design system from existing design.

Sure, we can unify the details at this stage

@IronLu233
Copy link

At first, color, the preliminary things for a design.
We should define several color tokens.
Like tailwindCSS default color, base color should be defined. and then, use this color scale tool to generate all scales.

Please ensure all colors can be found in the palette. @Kirl70

@Kirl70
Copy link

Kirl70 commented Jul 5, 2023

At first, color, the preliminary things for a design. We should define several color tokens. Like tailwindCSS default color, base color should be defined. and then, use this color scale tool to generate all scales.

Please ensure all colors can be found in the palette. @Kirl70

The design guidelines will be developed after the official website design is completed.

@Danie0918 Danie0918 assigned Danie0918 and unassigned IronLu233 Jul 17, 2023
@Kirl70
Copy link

Kirl70 commented Jul 27, 2023

@Danie0918 Danie0918 assigned Sven-TBD, Keith-CY and WhiteMinds and unassigned Kirl70 and Danie0918 Sep 4, 2023
@Danie0918 Danie0918 changed the title UI Guidelines UI Guidelines&Front-end refactoring Sep 4, 2023
@Danie0918
Copy link
Contributor

@Sven-TBD Please add chart scaling logic and other product solutions that need to be optimized.

@Keith-CY @WhiteMinds Please add a list of front-end code refactoring and options.

@PainterPuppets @zhangyouxin @Daryl-L If there are other CKB Explorer front-ends that need to be optimized, please add them.

@WhiteMinds
Copy link

Firstly, I would like to list the main goals of this refactoring: to improve the maintainability and scalability of the code, which will pave the way for future development.

To achieve this goal, we should make the existing code more standardized and cohesive (easy to read and easy to maintain). Here are some of my proposals:

  1. Internal types should not be placed in types/index.d.ts.

    1. This is not standard and behaves differently from regular ts files. https://stackoverflow.com/a/60092162/21858805

    2. This is not cohesive. Each type should be exported by the ts file responsible for maintaining that type.

      1. Internal types can be refactored into a separate models layer, where commonly used models and types can be implemented.

      2. External types can create corresponding services layers for each external service, where internal and external data exchange and type constraints can be implemented.

  2. The names of types such as AppState, App, and PageState need to be reconsidered, and their inheritance and composition relationships should also be adjusted. For example, AppState should not inherit from PageState.

  3. Some external service types are incorrect, such as State.Transaction. It does not match some interface return types, and some interfaces are missing some fields and need to be redesigned.

  4. Consider refactoring the existing global state code because the current implementation is a bit complex and cumbersome, and updating any global state will rerender all components that use useAppState.

    1. We need to explore the best practices for cohesive global state code within components.

      1. You can consider using some state management libraries, such as jotai, recoil, zustand, etc.
  5. Make the implementation of each component more cohesive.

    1. Resource cohesion, such as when an icon is only used by one component, it should be placed directly in the component. If it is used by two components, it can also be considered to be copied. Only when it is really a commonly used icon should it be placed in a common location.

    2. State cohesion, but it should not carry unnecessary state.

    3. Style cohesion, some styles should be implemented by the component itself as a switch, exposed to the outside through props for switching, rather than being modified by the outside. The outside should only modify position, size and scaling-related styles as much as possible.

    4. I hope i18n can also be more cohesive, but this requires feasibility and cost research.

    5. Ensure that only deleting a component's folder and the few lines of code that import and use it will cleanly remove it. Note that this is just a guideline, not a goal.

  6. Some folder and file locations and names need to be adjusted, such as contexts/providers/hook.ts, which is a bit strange.

  7. I18n should be used through hooks as much as possible so that it can respond in a timely manner when the language changes. Some places directly use the object exported in i18n.ts.

  8. Enable configuration options such as noUncheckedIndexedAccess to use the type system more strictly and consistently.

  9. Remove behaviors that break the type system as much as possible, such as using as, any, etc.

  10. Remove the use of !important in CSS as much as possible, as it can affect maintainability.

There are a few other ideas in the works.

@Keith-CY
Copy link
Member

Keith-CY commented Sep 4, 2023

Firstly, I would like to list the main goals of this refactoring: to improve the maintainability and scalability of the code, which will pave the way for future development.

To achieve this goal, we should make the existing code more standardized and cohesive (easy to read and easy to maintain). Here are some of my proposals:

  1. Internal types should not be placed in types/index.d.ts.

    1. This is not standard and behaves differently from regular ts files. stackoverflow.com/a/60092162/21858805

    2. This is not cohesive. Each type should be exported by the ts file responsible for maintaining that type.

      1. Internal types can be refactored into a separate models layer, where commonly used models and types can be implemented.
      2. External types can create corresponding services layers for each external service, where internal and external data exchange and type constraints can be implemented.
  2. The names of types such as AppState, App, and PageState need to be reconsidered, and their inheritance and composition relationships should also be adjusted. For example, AppState should not inherit from PageState.

  3. Some external service types are incorrect, such as State.Transaction. It does not match some interface return types, and some interfaces are missing some fields and need to be redesigned.

  4. Consider refactoring the existing global state code because the current implementation is a bit complex and cumbersome, and updating any global state will rerender all components that use useAppState.

    1. We need to explore the best practices for cohesive global state code within components.

      1. You can consider using some state management libraries, such as jotai, recoil, zustand, etc.
  5. Make the implementation of each component more cohesive.

    1. Resource cohesion, such as when an icon is only used by one component, it should be placed directly in the component. If it is used by two components, it can also be considered to be copied. Only when it is really a commonly used icon should it be placed in a common location.
    2. State cohesion, but it should not carry unnecessary state.
    3. Style cohesion, some styles should be implemented by the component itself as a switch, exposed to the outside through props for switching, rather than being modified by the outside. The outside should only modify position, size and scaling-related styles as much as possible.
    4. I hope i18n can also be more cohesive, but this requires feasibility and cost research.
    5. Ensure that only deleting a component's folder and the few lines of code that import and use it will cleanly remove it. Note that this is just a guideline, not a goal.
  6. Some folder and file locations and names need to be adjusted, such as contexts/providers/hook.ts, which is a bit strange.

  7. I18n should be used through hooks as much as possible so that it can respond in a timely manner when the language changes. Some places directly use the object exported in i18n.ts.

  8. Enable configuration options such as noUncheckedIndexedAccess to use the type system more strictly and consistently.

  9. Remove behaviors that break the type system as much as possible, such as using as, any, etc.

  10. Remove the use of !important in CSS as much as possible, as it can affect maintainability.

There are a few other ideas in the works.

I wonder if it's necessary to keep global state. IMO, the state goes across pages is latest block, which is used to calculate the distance of a specific page between the latest blockchain state, e.g. confirmations on the block page.

If so, latest block can be shared by use-query by cache instead of being maintained in global state manually.

@WhiteMinds
Copy link

I wonder if it's necessary to keep global state. IMO, the state goes across pages is latest block, which is used to calculate the distance of a specific page between the latest blockchain state, e.g. confirmations on the block page.

If so, latest block can be shared by use-query by cache instead of being maintained in global state manually.

We can indeed refactor and remove some of the global state, but there will still inevitably be some states that are truly global. If they all use Context provided by each component, it may be somewhat cumbersome, so it is still recommended to find a simpler way to implement components or services that provide global state externally.

@WhiteMinds
Copy link

To achieve this goal, we should make the existing code more standardized and cohesive (easy to read and easy to maintain). Here are some of my proposals:

  1. ...

The passage above mainly makes some practical suggestions.

In addition, I also hope the project can have some changes in development philosophy.

I think the current project lacks some core point sthat developers hoping to understand the project can use to quickly grasp it. Instead, various logics are mixed and scattered, making it hard to comprehend. This makes it difficult for people taking over the team or people in the community wanting to participate to get started.

So I hope to establish a core model layer, separating the model, external services, and views.

Some reference materials:

  1. https://herbertograca.com/2017/11/16/explicit-architecture-01-ddd-hexagonal-onion-clean-cqrs-how-i-put-it-all-together/#fundamental-blocks-of-the-system
  2. https://mp.weixin.qq.com/s/STYOtIj3s1IAvdaAX_JNhQ

Even if we don't do this now, we can first learn about it.

@Keith-CY
Copy link
Member

Keith-CY commented Sep 5, 2023

I wonder if it's necessary to keep global state. IMO, the state goes across pages is latest block, which is used to calculate the distance of a specific page between the latest blockchain state, e.g. confirmations on the block page.
If so, latest block can be shared by use-query by cache instead of being maintained in global state manually.

We can indeed refactor and remove some of the global state, but there will still inevitably be some states that are truly global. If they all use Context provided by each component, it may be somewhat cumbersome, so it is still recommended to find a simpler way to implement components or services that provide global state externally.

We can have a list of global states first since I merely use global states in godwoken blockchain explorer

@WhiteMinds
Copy link

After the view and data enter a decent decoupled state, we can consider introducing storybook. This will help us implement each component and its responsiveness more independently and better, and help promote some CSS development strategies, such as not allowing the component container itself to contain position, size, scaling and other styles, but having them set by the code using the component and only allowing the setting of such information.
The specific rules can be further discussed. This will help us better control the development of styles and their scope of influence, avoiding style bugs.

@WhiteMinds
Copy link

We can have a list of global states first since I merely use global states in godwoken blockchain explorer

The tipBlockNumber in State.Statistics can be replaced by State.App['tipBlockNumber']. Other properties are basically only used by Home, they can be refactored into Home, with State.Statistics['reorgStartedAt'] as an exception.

The language, primaryColor, secondaryColor, chartColor in State.App are constants that can be refactored and removed. The other 3 should be kept.

The 3 variables in State.Components look like globals that need to be retained.

@Keith-CY
Copy link
Member

Keith-CY commented Sep 5, 2023

We can have a list of global states first since I merely use global states in godwoken blockchain explorer

The tipBlockNumber in State.Statistics can be replaced by State.App['tipBlockNumber']. Other properties are basically only used by Home, they can be refactored into Home, with State.Statistics['reorgStartedAt'] as an exception.

The language, primaryColor, secondaryColor, chartColor in State.App are constants that can be refactored and removed. The other 3 should be kept.

The 3 variables in State.Components look like globals that need to be retained.

I've checked State.App, it's

export interface App {
    toast: ToastMessage | null
    appErrors: [
      { type: 'Network'; message: string[] },
      { type: 'ChainAlert'; message: string[] },
      { type: 'Maintenance'; message: string[] },
    ]
    tipBlockNumber: number
    language: 'en' | 'zh'
    primaryColor: string
    secondaryColor: string
    chartColor: {
      ...
    }
  }

So the other 3 should be kept must mean toast, appErrors, and tipBlockNumber.

Among them, tip block number can be avoided by using the cached tip block number of a request.

toast can be encapsulated into an independent Toast component`

appErrors: we may need to connect to the backend developers to know when these errors will be used. AFAIK Maintenance is not used anymore because most time the entire server is down, so no signals will be returned from the maintanence API.

Then the State.Component is

  export interface Components {
    mobileMenuVisible: boolean
    headerSearchBarVisible: boolean
    maintenanceAlertVisible: boolean
  }

By reading the code I found they are just used for responsible layout, and can be avoided by using css.

@WhiteMinds
Copy link

It looks like it's possible to try to remove all of them as much as possible during the refactoring effort.

@Keith-CY
Copy link
Member

Keith-CY commented Sep 6, 2023

We can have a list of global states first since I merely use global states in godwoken blockchain explorer

The tipBlockNumber in State.Statistics can be replaced by State.App['tipBlockNumber']. Other properties are basically only used by Home, they can be refactored into Home, with State.Statistics['reorgStartedAt'] as an exception.
The language, primaryColor, secondaryColor, chartColor in State.App are constants that can be refactored and removed. The other 3 should be kept.
The 3 variables in State.Components look like globals that need to be retained.

I've checked State.App, it's

export interface App {
    toast: ToastMessage | null
    appErrors: [
      { type: 'Network'; message: string[] },
      { type: 'ChainAlert'; message: string[] },
      { type: 'Maintenance'; message: string[] },
    ]
    tipBlockNumber: number
    language: 'en' | 'zh'
    primaryColor: string
    secondaryColor: string
    chartColor: {
      ...
    }
  }

So the other 3 should be kept must mean toast, appErrors, and tipBlockNumber.

Among them, tip block number can be avoided by using the cached tip block number of a request.

toast can be encapsulated into an independent Toast component`

appErrors: we may need to connect to the backend developers to know when these errors will be used. AFAIK Maintenance is not used anymore because most time the entire server is down, so no signals will be returned from the maintanence API.

Then the State.Component is

  export interface Components {
    mobileMenuVisible: boolean
    headerSearchBarVisible: boolean
    maintenanceAlertVisible: boolean
  }

By reading the code I found they are just used for responsible layout, and can be avoided by using css.

For appErrors

  1. ChainAlert: used to display alert messages from /statistics/blockchain_info API, e.g. CKB v0.105.* have bugs. Please upgrade to the latest version., so it can be encapsulated into an independent component, specifically for this API

  2. Network: will be displayed when a API request fails. I have no idea if it should be managed separately or in global state. Need advice from others

  3. Maintenance: as mentioned above, it can be done by re-deployment on vercel with different runtime environment. So the state can be removed

@Keith-CY
Copy link
Member

Keith-CY commented Sep 8, 2023

We may split the tasks if there is no more feedback from @WhiteMinds @zhangyouxin @PainterPuppets @Daryl-L

@Sven-TBD Sven-TBD added the UserStory a brief, plain-language explanation of a feature or functionality label Nov 27, 2023
@Sven-TBD
Copy link
Contributor Author

@WhiteMinds How is it going with these issues of refactor?

@WhiteMinds
Copy link

@WhiteMinds How is it going with these issues of refactor?

I didn't have time to work on this task last week, and it's unlikely that I'll have time this week either.

This could potentially be a long-term and large task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor UserStory a brief, plain-language explanation of a feature or functionality
Projects
Status: 🏗 In Progress
Development

No branches or pull requests

8 participants