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

fix styles for hub pages and article page #9765

Merged
merged 39 commits into from
Jul 13, 2022
Merged

Conversation

marcochavezf
Copy link
Contributor

@marcochavezf marcochavezf commented Jul 7, 2022

cc @roryabraham @michelle-thompson

Details

This PR includes mainly style changes for the Help site to mimic the mockups design from the design doc.

Some other minor changes included in this PR:

  • Update outdated links from comments in the README.md file
  • Rename files from the _includes folder (changed -navigation-tree suffix to -lhn)
  • Add functionality to toggle the menu in small views (mobile web)

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/216742

Tests

  1. (you need ruby and bundler installed to do this, more info here)

  2. Run:

    cd docs
    bundle install
    bundle exec jekyll serve
  3. Make sure the site's styles load and that all the colors look correct.

Additionally add more content like Stage titles, Groups of cards and card buttons to one of the hub pages (i.e. request-money and Stage title anchors and Article links to the respective LHN section (i.e. request-money-lhn).

Screenshots

Hub page

Desktop (updated):
localhost_4000_hubs_request-money_

Mobile (updated):

Article page

Desktop:
localhost_4000_articles_request-money_SmartScan

Mobile:

@marcochavezf marcochavezf self-assigned this Jul 7, 2022
@marcochavezf marcochavezf changed the title [WIP] fix hub pages 2 [WIP] fix hub pages Jul 7, 2022
@marcochavezf marcochavezf changed the title [WIP] fix hub pages fix styles for hub pages and article page Jul 9, 2022
@marcochavezf marcochavezf marked this pull request as ready for review July 9, 2022 22:24
@marcochavezf marcochavezf requested a review from a team as a code owner July 9, 2022 22:24
@melvin-bot melvin-bot bot requested review from MariaHCD and removed request for a team July 9, 2022 22:24
Copy link
Contributor

@MariaHCD MariaHCD left a comment

Choose a reason for hiding this comment

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

Looks great! Just a few comments.

docs/hubs/request-money.html Show resolved Hide resolved
docs/_includes/home-lhn.html Outdated Show resolved Hide resolved
docs/_layouts/default.html Show resolved Hide resolved
@michelle-thompson
Copy link
Contributor

image

The icon is the wrong color - it should be the same as the icons in the LHN. Additionally, the border is too dark - let's change it to #ECECEC

@marcochavezf
Copy link
Contributor Author

image

The icon is the wrong color - it should be the same as the icons in the LHN. Additionally, the border is too dark - let's change it to #ECECEC

Good catch @michelle-thompson! I updated the styles for the card in the hub pages and also updated the screenshots in the PR description.

@MariaHCD I addressed the requested changes, thanks for the review! 🙇🏽 The PR is ready to be reviewed again.

@michelle-thompson
Copy link
Contributor

One more comment, I noticed the hyperlinks are medium weight. Links should be regular weight (unless the body text is bolded, in which case it should follow the body text)
image

@marcochavezf
Copy link
Contributor Author

marcochavezf commented Jul 12, 2022

One more comment, I noticed the hyperlinks are medium weight. Links should be regular weight (unless the body text is bolded, in which case it should follow the body text) image

@michelle-thompson Ah yes, it was a bolded hyperlink, I think I accidentally added the bold property in the article .md content. This how the link looks like without the bold property:

localhost_4000_articles_request-money_SmartScan (1)

@michelle-thompson
Copy link
Contributor

Looks great, thank you!

@ctkochan22
Copy link
Contributor

Reviewing the other PR: #9869 (comment)

It looks like we are good right? It seems @michelle-thompson gave it the thumbs up.

@ctkochan22 ctkochan22 merged commit 0c5cf60 into main Jul 13, 2022
@ctkochan22 ctkochan22 deleted the marco-fixHubPages-2 branch July 13, 2022 15:31
@OSBotify
Copy link
Contributor

🚀 Deployed to production by @chiragsalian in version: 1.1.84-13 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Comment on lines +200 to +202
.header-button {
position: absolute;
display: none;
Copy link
Contributor

Choose a reason for hiding this comment

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

This caused #25646 down the road.

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.

6 participants