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

doc: fix bullet(arrow) alignment vertically in documentation #52193

Merged
merged 2 commits into from Mar 27, 2024

Conversation

akashyeole
Copy link
Contributor

Fixed the alignment of the bullet points (green arrow) under 'Node.js documentation' by adjusting the property in style.css

Current:
image

Fix:
image

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/nodejs-website

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 23, 2024
Copy link
Contributor

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

LGTM ! Thanks for your first contribution

Copy link
Contributor

@aduh95 aduh95 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 PR! I don't think using position relative is a good choice, as it's going to depend on which fonts are supported by the client.

Here's what it looks like for me without this PR:
image

Here's how it looks with this PR:
image

Maybe a better fix would be to build the triangle in CSS using ::after and ::before so it can look the same independently of the client fonts.

@akashyeole
Copy link
Contributor Author

@aduh95 I got this. I will do the changes 👍

@akashyeole akashyeole requested a review from aduh95 March 23, 2024 15:36
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

Looks good! I would have a slight preference that we drop that .picker-arrow empty span (to use ::before instead), but LGTM regardless.

@akashyeole
Copy link
Contributor Author

@aduh95 I have updated the commit message (col <72), the check won't fail now.

@akashyeole
Copy link
Contributor Author

Hi @aduh95 can you please approve the workflows 😄

@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 27, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 27, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/52193
✔  Done loading data for nodejs/node/pull/52193
----------------------------------- PR info ------------------------------------
Title      doc: fix bullet(arrow) alignment vertically in documentation (#52193)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     akashyeole:akashyeole -> nodejs:main
Labels     doc
Commits    2
 - doc: fix bullet(arrow) alignment vertically
 - doc,tools: replace bullet triangle with pure css design
Committers 1
 - Akash Yeole 
PR-URL: https://github.com/nodejs/node/pull/52193
Reviewed-By: Antoine du Hamel 
Reviewed-By: Luigi Pinca 
Reviewed-By: Moshe Atlow 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/52193
Reviewed-By: Antoine du Hamel 
Reviewed-By: Luigi Pinca 
Reviewed-By: Moshe Atlow 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 23 Mar 2024 07:13:46 GMT
   ✔  Approvals: 3
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/52193#pullrequestreview-1956435636
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/52193#pullrequestreview-1956571422
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/52193#pullrequestreview-1959678422
   ⚠  GitHub cannot link the author of 'doc: fix bullet(arrow) alignment vertically' to their GitHub account.
   ⚠  Please suggest them to take a look at https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork
   ⚠  GitHub cannot link the author of 'doc,tools: replace bullet triangle with pure css design' to their GitHub account.
   ⚠  Please suggest them to take a look at https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 52193
From https://github.com/nodejs/node
 * branch                  refs/pull/52193/merge -> FETCH_HEAD
✔  Fetched commits as 38161c38d9e6..be7b4145a59f
--------------------------------------------------------------------------------
[main 53cf099b71] doc: fix bullet(arrow) alignment vertically
 Author: Akash Yeole 
 Date: Sat Mar 23 12:31:09 2024 +0530
 1 file changed, 2 insertions(+)
[main 3bfcb8c879] doc,tools: replace bullet triangle with pure css design
 Author: Akash Yeole 
 Date: Sat Mar 23 21:00:52 2024 +0530
 3 files changed, 29 insertions(+), 26 deletions(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: fix bullet(arrow) alignment vertically

Fixed the alignment of the bullet points (green arrow) under
'Node.js documentation' by adjusting the property in
style.css

PR-URL: #52193
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Moshe Atlow moshe@atlow.co.il

[detached HEAD 0cf52ef30f] doc: fix bullet(arrow) alignment vertically
Author: Akash Yeole akashyeole02@gmail.co
Date: Sat Mar 23 12:31:09 2024 +0530
1 file changed, 2 insertions(+)
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc,tools: replace bullet triangle with pure css design

Removed the hardcoded triangle from doc/template.html and
tools/doc/html.mjs and placed the new element which
takes the shape of triangle using the new style defined in css.

PR-URL: #52193
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Moshe Atlow moshe@atlow.co.il

[detached HEAD dc4c2eaeb1] doc,tools: replace bullet triangle with pure css design
Author: Akash Yeole akashyeole02@gmail.co
Date: Sat Mar 23 21:00:52 2024 +0530
3 files changed, 29 insertions(+), 26 deletions(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/8458019623

Akash Yeole added 2 commits March 28, 2024 02:13
Fixed the alignment of the bullet points (green arrow) under
'Node.js <version> documentation' by adjusting the property in
style.css
Removed the hardcoded triangle from doc/template.html and
tools/doc/html.mjs and placed the new <span> element which
takes the shape of triangle using the new style defined in css.
@akashyeole
Copy link
Contributor Author

@lpinca
The previous 'commit-queue' failed because of incorrect author email in config, I just updated my email in the PR.

@lpinca lpinca added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Mar 27, 2024
@aduh95 aduh95 merged commit 27493a1 into nodejs:main Mar 27, 2024
23 checks passed
@aduh95
Copy link
Contributor

aduh95 commented Mar 27, 2024

Landed in 27493a1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants