Frontmatter
| title | Improve feature card and layout in the homepage |
| author | VedantDewangan |
| state | Merged |
| createdAt | Oct 1, 2024, 1:27 PM |
| updatedAt | Oct 1, 2024, 10:38 PM |
| closedAt | Oct 1, 2024, 10:38 PM |
| mergedAt | Oct 1, 2024, 10:38 PM |
| branches | dev ← feature/improve-feature-card-homepage |
| url | https://github.com/neomjs/neo/pull/5996 |
hi @VedantDewangan,
one big wish: could you please keep the current code-formatting? More precisely the block formatting (align on colon). you can enable this in most IDEs. Otherwise it gets super hard to track what really did change: https://github.com/neomjs/neo/pull/5996/files
I am super busy right now, but will take a closer look at the content within the next 24h.
Thx and best regards, Tobi
Hi @tobiu,
Thank you for your feedback! I appreciate your insights on maintaining the code formatting for better readability. This is my first pull request, and I'm eager to improve and learn from this experience.
If there are any additional changes you would like me to make, please let me know, and I will definitely work on them. I believe it will help enhance the overall quality of the project.
To make it easier to see the changes, I have prepared two short videos showing the before and after states of the code formatting. I will attach them below for your review.
This is what the design looked like before changes: Before Video
This is after the change: After Video
As this is my first pull request, I would be incredibly grateful if you could consider accepting it. It would mean a lot to me and help me achieve my first Hacktoberfest contribution!
Thank you for your time, and I look forward to your suggestions!
Best regards
hi @VedantDewangan,
first of all: I will approve the PR => the goal is to make first contributions easy.
after taking a closer look, some comments:
- coding style: we are using 4 spaces inside JS & SCSS files (i will fix this one)
- coding style: we are ordering items alphabetically inside JS & SCSS files (i will fix this one too)
there are coding guidelines out there, but a bit hidden: https://github.com/neomjs/neo/blob/dev/.github/CODING_GUIDELINES.md
=> adding the SCSS guidelines in there would be worth another ticket / PR => also, defining eslint & prettier specs which honor the current coding guidelines as good as possible would be worth a ticket & PR (we tried it inside a client project, but it is non-trivial)
- we can not change the
font-familyjust inside one local spot. consistency. @mxmrtns - the same goes for the scrollbar definitions: either everywhere the same (for one theme) or better leave it out
- is there a reason to replace the grid layout with display flex? i need to test this on different resolutions
- avoid using
!importantinside CSS => this should be the last resort if nothing else works, since it makes overrides in class extensions very tricky - list items hover effect: from a UX perspective this only makes sense, in case a user can interact with the items (e.g. a click action). In this case we would use
list.Baseinstead of a pureul, limarkup => then we get a selection model, keyboard navigation & other goodies.
Thanks for the PR and welcome to the contributors list!
Best regards, Tobi
Please make sure to read the Contributing Guidelines:
https://github.com/neomjs/neo/blob/dev/CONTRIBUTING.md
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
devbranch, not themainbranchfix #xxx[,#xxx], where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information: