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

A Material Design Makeover for Substrate Front-End Template #2159

Merged
merged 8 commits into from
Jan 24, 2024

Conversation

stojanov-igor
Copy link
Contributor

Project Abstract

This project aims to enhance the user experience of the substrate-front-end-template by migrating to Material Design UI, ensuring a visually appealing, intuitive, and modern interface that aligns with contemporary design standards.

Grant level

  • Level 1: Up to $10,000, 2 approvals
  • Level 2: Up to $30,000, 3 approvals
  • Level 3: Unlimited, 5 approvals (for >$100k: Web3 Foundation Council approval)

Application Checklist

  • The application template has been copied and aptly renamed (project_name.md).
  • I have read the application guidelines.
  • Payment details have been provided (bank details via email or Polkadot (USDC & USDT) address in the application).
  • I am aware that, in order to receive a grant, I (and the entity I represent) have to successfully complete a KYC/KYB check.
  • The software delivered for this grant will be released under an open-source license specified in the application.
  • The initial PR contains only one commit (squash and force-push if needed).
  • The grant will only be announced once the first milestone has been accepted (see the announcement guidelines).
  • I prefer the discussion of this application to take place in a private Element/Matrix channel. My username is: @_______:matrix.org (change the homeserver if you use a different one)

Copy link
Contributor

github-actions bot commented Dec 20, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@stojanov-igor
Copy link
Contributor Author

I have read and hereby sign the Contributor License Agreement.

Copy link
Contributor

@keeganquigley keeganquigley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the application @stojanov-igor it would be nice to see this worked on again but I have a few questions:

  • Material 3 is pretty cool. Have you looked into Polkadot Cloud? Since it is frame-agnostic, I'm wondering if you could integrate the library to keep the themes/aesthetics in line with other Polkadot apps such as the staking dashboard.
  • Regarding M1, we usually don't fund the initial designs. Could you remove that deliverable?
  • Because of this, we typically ask for mock-ups and/or figma designs to be provided upfront. Could you add these even if they are a rough draft?

@keeganquigley keeganquigley added the changes requested The team needs to clarify a few things first. label Dec 20, 2023
@keeganquigley keeganquigley self-assigned this Dec 20, 2023
@stojanov-igor
Copy link
Contributor Author

Hi @keeganquigley

Material 3 is pretty cool. Have you looked into Polkadot Cloud? Since it is frame-agnostic, I'm wondering if you could integrate the library to keep the themes/aesthetics in line with other Polkadot apps such as the staking dashboard.

I do agree that using the common theme colors is ideal. I will integrate the polkadot cloud themes.

Regarding M1, we usually don't fund the initial designs. Could you remove that deliverable?

Removed.

Because of this, we typically ask for mock-ups and/or figma designs to be provided upfront. Could you add these even if they are a rough draft?

Added an initial draft to the original proposal application.

@muddlebee
Copy link
Contributor

It will be nice to see Angular + Material templates for the Substrate Front-End Template and Polkadot Cloud stuff :)

@keeganquigley keeganquigley added ready for review The project is ready to be reviewed by the committee members. and removed changes requested The team needs to clarify a few things first. labels Jan 9, 2024
keeganquigley
keeganquigley previously approved these changes Jan 9, 2024
Copy link
Contributor

@keeganquigley keeganquigley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay @stojanov-igor and thanks for making the changes. Given the cost and the fact that we had to close a previous grant for this, I'm willing to go ahead with it.

While waiting for others to comment, we also started requiring KYC/KYB checks for all potential grantees. Could you please provide the information outlined under this link? Let me know if you have any questions or issues!

@keeganquigley keeganquigley added the admin-review This application requires a review from an admin. label Jan 9, 2024
Copy link
Collaborator

@takahser takahser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stojanov-igor thanks, this looks interesting. I've got a few questions:

  • are you going to use any existing frontend library for the implementation?
  • are you sure you'd need 2 months full-time to implement this relatively simply UI changes, i.e. replacing all UI elements with their Material-based counterparts? I've taken a brief look at the code and I have the impression that it could be rather quick and easy. Am I missing something here, i.e. is there anything that requires a bit more attention and is more complex than just replacing some UI elements and wiring up their props/state hooks?
  • would you be open to rewrite the components to TypeScript as part of this grant?
  • are you going to fully overhaul the template and use material for any UI component, or are there any exceptions?
  • finally, feel free to have a look at my inline comments. There I'm mainly interested if these features you list bring any new or updated functionality, or if the scope is limited to UI element updates.

| 1. | Cards Module | Implement dashboard cards with basic statistics about the blockchain. |
| 2. | Table Module| Add table that holds Account balances that can be loaded from Polkadot Extension |
| 3. | Transfer Module | Simple module to transfer tokens from one acount to another |
| 4. | Upgrade Runtime Module| EEnsure users can upload wasm File from the UI |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| 4. | Upgrade Runtime Module| EEnsure users can upload wasm File from the UI |
| 4. | Upgrade Runtime Module| Ensure users can upload a wasm File from the UI that allows them to upgrade their runtime |

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a new feature?

applications/si-front-end-template.md Show resolved Hide resolved
applications/si-front-end-template.md Show resolved Hide resolved
applications/si-front-end-template.md Show resolved Hide resolved
@semuelle semuelle removed the admin-review This application requires a review from an admin. label Jan 10, 2024
@stojanov-igor
Copy link
Contributor Author

Hi @keeganquigley

Thanks for your comments. I have already completed a KYC. @semuelle requested some changes that I have already incorporated in the proposal.

@takahser Thanks for the minor improvements. You will find answers to your questions below.

are you going to use any existing frontend library for the implementation?

Yes. I will use individual web components from the mui framework. @mui/material @emotion/react @emotion/styled

are you sure you'd need 2 months full-time to implement this relatively simply UI changes, i.e. replacing all UI elements with their Material-based counterparts? I've taken a brief look at the code and I have the impression that it could be rather quick and easy. Am I missing something here, i.e. is there anything that requires a bit more attention and is more complex than just replacing some UI elements and wiring up their props/state hooks?

Based on my analysis there are about 20 distinct components that have to be restyled in the Material Design Style Guide, so the estimated 2-month timeframe is appropriate. Some of the existing components (such as the top dashboard cards) don't have an exact implementation in Material Design, so it would require custom work. As mentioned in the proposal, there is
a new functionality that adds themes.

would you be open to rewrite the components to TypeScript as part of this grant?

No

are you going to fully overhaul the template and use material for any UI component, or are there any exceptions?
finally, feel free to have a look at my inline comments. There I'm mainly interested if these features you list bring any new or updated functionality, or if the scope is limited to UI element updates.

It will be a full overhaul of the template based on Material Design Standards. In the case when these standards are not provided, I will provide custom styling that is closest to the provided standard. Most of the features that are listed already exist, so it will be mostly UI updates to modernize the front-end template in hopes of enabling new upcoming developers a better user experience when building and working with Substrate.

Copy link
Collaborator

@takahser takahser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stojanov-igor thanks for your reply. In that case I'm going to withhold my vote for now, the reasons being:

  • I feel like 2 month is a lot to replace some ui-components + some props/state hook updates with little additional logic
  • regarding the top dashboard card: I think the only thing missing is the horizontal line, which should be easy to add: you can just add a <hr /> HTML element or set the CSS border attribute accordingly.
  • While I applaud the idea of giving the template a fresh look, I believe a rewrite to TypeScript would be useful as well, since it would introduce better type and code safety, IDE support, readability, etc. In case you change your mind on this, I'd be open to support the grant.
  • Themes are nice but I'm not sure if it's very useful, since this template is used for the very early-stages of a Polkadot SDK project, rather than a public/customer-facing product.

Copy link
Contributor

@keeganquigley keeganquigley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanations @stojanov-igor I do agree with my colleague that TypeScript would be better, and this might help to change the minds of other committee members. If not willing to switch, would you be able to reduce the scope/cost a bit?

Copy link
Member

@semuelle semuelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates, @stojanov-igor. I have unresolved two issues from @takahser above, as it seems to me that the questions weren't answered.
My main question - maybe I missed the answer in your document - would be whether you are planning to have these changes merged into the substrate-front-end-template repo or whether you are planning to maintain a fork. For the former, have you reached out to the maintainers, (e.g. @sacha-l) if this is something they'd be interested in?

@takahser
Copy link
Collaborator

@semuelle regarding the issues you've unresolved above - I think the features mostly exist already, quote from @stojanov-igor's comment:

Most of the features that are listed already exist, so it will be mostly UI updates to modernize the front-end template in hopes of enabling new upcoming developers a better user experience when building and working with Substrate.

@stojanov-igor
Copy link
Contributor Author

Hi all,

My comments/ answers are below.

would be whether you are planning to have these changes merged into the substrate-front-end-template repo or whether you are planning to maintain a fork. For the former, have you reached out to the maintainers, (e.g. @sacha-l) if this is something they'd be interested in?

The plan is to maintain a fork of the official substrate-front-end repository under the same open and permissive license. We can create a pull request from the fork to the official repository if the maintainers would like to adopt my changes.

Thanks for the explanations @stojanov-igor I do agree with my colleague that TypeScript would be better, and this might help to change the minds of other committee members.

If the community prefers a typescript version, we will do the conversion to typescript as part of the delivery. Some old school javascript developers dislike strict typing and type annotations but if typescript is a strong preference for future development, we are willing to provide a typescript version.

I feel like 2 month is a lot to replace some ui-components + some props/state hook updates with little additional logic
regarding the top dashboard card: I think the only thing missing is the horizontal line, which should be easy to add: you can just add a HTML element or set the CSS border attribute accordingly.
While I applaud the idea of giving the template a fresh look, I believe a rewrite to TypeScript would be useful as well, since it would introduce better type and code safety, IDE support, readability, etc. In case you change your mind on this, I'd be open to support the grant.
Themes are nice but I'm not sure if it's very useful, since this template is used for the very early-stages of a Polkadot SDK project, rather than a public/customer-facing product.

Thank you for expressing your opinion @takahser. I think you are underestimating the work required. For instance, if you only look at the pallet Interactor you will notice that these are not simple components. The fact that the code has not been updated in more than 2 years is also worrying for those who want to build applications on polkadot.

As mentioned above, we can provide a typescript rewrite if this is what the development community wants.

@takahser
Copy link
Collaborator

@stojanov-igor thanks for your reply

if you only look at the pallet Interactor you will notice that these are not simple components.

Pls correct me if I'm wrong, but wouldn't you just have to update the return value with the new UI components? Meaning, you'd bind all the event handlers of the existing component to the new ones (e.g. onChange={onInterxTypeChange}) and you're done.

As mentioned above, we can provide a typescript rewrite if this is what the development community wants.

That would be lovely indeed. I'd support the grant, if a complete TypeScript rewrite was included.

@stojanov-igor
Copy link
Contributor Author

Hi all,

I have added Typescript migration as part of the milestone 1.

Pls correct me if I'm wrong, but wouldn't you just have to update the return value with the new UI components? Meaning, you'd bind all the event handlers of the existing component to the new ones (e.g. onChange={onInterxTypeChange}) and you're done.

The template is currently using semantic-ui-react. Migrating to Material-Ui is a little more complex than changing the attributes to new ones due to different design philosophies of Semantic UI and Material UI. For instance, the Interactor component that I mentioned previously is using Grid, Form, Dropdown, Input, Label which might not have the same attributes within Material Design. Furthermore, there are custom components that the substrate team has implemented such as TxButton, TxGroupButton within substrate-lib which have to be restyled manually.

Also, Material-UI follows a tree-shaking approach for importing components which allows developers to selectively import and use only the needed components. Semantic UI has a pre-build bundle which is larger in-size and can negatively affect performance.

Hope that clears out the confusion.

Copy link
Contributor

@keeganquigley keeganquigley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for being willing to switch to TS, happy to re-approve.

Copy link
Collaborator

@takahser takahser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, Material-UI follows a tree-shaking approach for importing components which allows developers to selectively import and use only the needed components. Semantic UI has a pre-build bundle which is larger in-size and can negatively affect performance.

I understand that there are some design differences, but the library already comes with it, right? Sure, you'd have to adjust some imports, but I don't see how that would significantly increase the workload. But anyway, since you've added the TypeScript rewrite I'm fine with approving this as well.

@takahser takahser merged commit d30df07 into w3f:master Jan 24, 2024
7 of 8 checks passed
Copy link
Contributor

Congratulations and welcome to the Web3 Foundation Grants Program! Please refer to our Milestone Delivery repository for instructions on how to submit milestones and invoices, our FAQ for frequently asked questions and the support section of our README for more ways to find answers to your questions.

Before you start, take a moment to read through our announcement guidelines for all communications related to the grant or make them known to the right person in your organisation. In particular, please don't announce the grant publicly before at least the first milestone of your project has been approved. At that point or shortly before, you can get in touch with us at [email protected] and we'll be happy to collaborate on an announcement about the work you’re doing.

Lastly, please remember to let us know in case you run into any delays or deviate from the deliverables in your application. You can either leave a comment here or directly request to amend your application via PR. We wish you luck with your project! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review The project is ready to be reviewed by the committee members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants