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

add Learneth plugin #4466

Merged
merged 15 commits into from
Feb 21, 2024
Merged

add Learneth plugin #4466

merged 15 commits into from
Feb 21, 2024

Conversation

drafish
Copy link
Contributor

@drafish drafish commented Jan 17, 2024

Remix LearnEth Plugin

Available Scripts

In the project directory, you can run:

npm run serve:plugin --plugin=learneth

Runs the app in the development mode.
Open http://localhost:2024 to view it in the browser.

The page will reload if you make edits.
You will also see any lint errors in the console.

npm run build:plugin --plugin=learneth

Builds the app for production to the dist/apps/learneth folder.

You can test it on chinese mirror site of remix ide -- https://remix.learnblockchain.cn/?#activate=LearnEth

Here is the url of learneth on mirror site -- https://remix.learnblockchain.cn/plugins/learneth/index.html

Copy link

netlify bot commented Jan 17, 2024

👷 Deploy request for remixproject accepted.

Name Link
🔨 Latest commit fd76c4e
🔍 Latest deploy log https://app.netlify.com/sites/remixproject/deploys/65d5ffcaf71b1900080b9edd

Copy link
Collaborator

@ryestew ryestew left a comment

Choose a reason for hiding this comment

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

From my point of view, the plugin is working.

@drafish drafish force-pushed the learneth branch 2 times, most recently from e1fd9bb to 49d2d36 Compare January 31, 2024 02:47
Copy link
Collaborator

Choose a reason for hiding this comment

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

we have it in assets folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to import the logo like this

import logo from '../../../../remix-ide/src/assets/img/remixLogo.webp'

Then I got this error when I was building
image

I tried this way which works, check below

<img src="https://remix.ethereum.org/assets/img/remixLogo.webp" />

But it's not elegant. Will that be ok? @LianaHus

Copy link
Collaborator

Choose a reason for hiding this comment

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

webp is also fine. depending on where you use it. does it work for both types of themes?

Copy link
Contributor Author

@drafish drafish Feb 2, 2024

Choose a reason for hiding this comment

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

I deleted the assets folder in learneth, get the assets from remix-ide folder. And yes, it works for both types of themes.

Copy link
Collaborator

@LianaHus LianaHus left a comment

Choose a reason for hiding this comment

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

Please, whatever is possible, do not use custom CSS classes but bootstrap once.

@drafish
Copy link
Contributor Author

drafish commented Feb 1, 2024

Please, whatever is possible, do not use custom CSS classes but bootstrap once.

What you mean by bootstrap once? Are you suggesting that use bootstrap classes only, and not use custom classes?

BTW, I got the custom CSS classes from the old learneth plugin. @LianaHus

to={`/list?id=${item.id}`}
className="text-decoration-none float-right"
>
<FontAwesomeIcon icon={faPlayCircle} size="lg" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I resaved all files. Now it's using remix's coding style.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm sorry I still see FontAwesomeIcon in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry I still see FontAwesomeIcon in the code.

just removed. please check again. thanks @LianaHus

display: flex;
flex-direction: column;
align-items: center;

Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove empty spaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed here, and other files too.

@LianaHus
Copy link
Collaborator

LianaHus commented Feb 1, 2024

Please, whatever is possible, do not use custom CSS classes but bootstrap once.

Please, whatever is possible, do not use custom CSS classes but bootstrap once.

What you mean by bootstrap once? Are you suggesting that use bootstrap classes only, and not use custom classes?

BTW, I got the custom CSS classes from the old learneth plugin. @LianaHus

yes. we are removing custom classes everywhere and using bootstrap classes

@drafish
Copy link
Contributor Author

drafish commented Feb 2, 2024

Please, whatever is possible, do not use custom CSS classes but bootstrap once.

Please, whatever is possible, do not use custom CSS classes but bootstrap once.

What you mean by bootstrap once? Are you suggesting that use bootstrap classes only, and not use custom classes?
BTW, I got the custom CSS classes from the old learneth plugin. @LianaHus

yes. we are removing custom classes everywhere and using bootstrap classes

Can I do this in next PR? There are a lot custom css classes that need to be removed. That may take a while. @LianaHus

@yann300 yann300 requested review from LianaHus and ryestew February 12, 2024 16:20
@drafish drafish force-pushed the learneth branch 2 times, most recently from d0fb83e to 8d6b879 Compare February 13, 2024 11:12
Copy link
Collaborator

@LianaHus LianaHus left a comment

Choose a reason for hiding this comment

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

Approving the PR. Please submit a new one to remove custom CSS and use bootstrap classes instead.

@yann300 yann300 merged commit f49961e into ethereum:master Feb 21, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants