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

feat: Added ArticleNavigation component, which provides next/prev navigation between articles #159

Merged
merged 6 commits into from
Feb 1, 2024

Conversation

LehaRybkoha
Copy link
Contributor

No description provided.

@LehaRybkoha LehaRybkoha self-assigned this Jan 16, 2024
src/components/ArticleNavigations/ArticleNavigations.tsx Outdated Show resolved Hide resolved
src/components/ArticleNavigations/ArticleNavigations.tsx Outdated Show resolved Hide resolved
src/components/Component/Component.tsx Outdated Show resolved Hide resolved
src/components/Component/Component.tsx Outdated Show resolved Hide resolved
src/pages/components/[libId]/[componentId].tsx Outdated Show resolved Hide resolved
src/pages/design/[sectionId]/[articleId].tsx Outdated Show resolved Hide resolved
src/components/Component/Component.tsx Outdated Show resolved Hide resolved
src/pages/design/[sectionId]/[articleId].tsx Outdated Show resolved Hide resolved
@chervyakovru
Copy link

chervyakovru commented Jan 20, 2024

  1. Cant open app for start review at all
Screenshot 2024-01-20 at 4 50 12 PM 2. At the first page buttons not exist image 3. height of buttons in code not equals with height of buttons in figma (66 vs 64) 4. there is no word "Navigations" in english. Lets use just "Navigation" 5. In the mobile version of the app, the buttons move off the screen and bring horizontal scrolling with them

src/components/ArticleNavigations/ArticleNavigations.scss Outdated Show resolved Hide resolved
src/components/ArticleNavigations/ArticleNavigations.scss Outdated Show resolved Hide resolved
src/components/ArticleNavigations/ArticleNavigations.scss Outdated Show resolved Hide resolved
src/components/ArticleNavigations/ArticleNavigations.scss Outdated Show resolved Hide resolved
src/components/ArticleNavigations/ArticleNavigations.scss Outdated Show resolved Hide resolved
src/components/ArticleNavigations/ArticleNavigations.scss Outdated Show resolved Hide resolved
src/components/ArticleNavigations/ArticleNavigations.scss Outdated Show resolved Hide resolved
src/components/ArticleNavigations/ArticleNavigations.scss Outdated Show resolved Hide resolved
src/components/ArticleNavigations/ArticleNavigations.scss Outdated Show resolved Hide resolved
src/components/ArticleNavigations/ArticleNavigations.tsx Outdated Show resolved Hide resolved
src/components/ArticleNavigation/ArticleNavigation.scss Outdated Show resolved Hide resolved
src/components/ArticleNavigation/ArticleNavigation.scss Outdated Show resolved Hide resolved
src/components/Component/Component.scss Outdated Show resolved Hide resolved
src/components/DesignArticle/DesignArticle.scss Outdated Show resolved Hide resolved
src/components/DesignArticle/DesignArticle.tsx Outdated Show resolved Hide resolved
src/pages/design/[sectionId]/[articleId].tsx Show resolved Hide resolved
@chervyakovru
Copy link

@imsitnikov Could u review the pr again, please?

@imsitnikov
Copy link
Collaborator

imsitnikov commented Jan 30, 2024

320px screen size:

image

@LehaRybkoha @chervyakovru Can we hide text into buttons on such a small screen size?

@LehaRybkoha
Copy link
Contributor Author

320px screen size:

image @LehaRybkoha @chervyakovru Can we hide text into buttons on such a small screen size?

No. It was made according to web-design. It will be fixed after integrating responsive version of landing. If you remove horizontal paddings, buttons will be fine and in responsive version paddings will be removed

@imsitnikov
Copy link
Collaborator

No. It was made according to web-design. It will be fixed after integrating responsive version of landing. If you remove horizontal paddings, buttons will be fine and in responsive version paddings will be removed

What is "responsive version of landing"?
Every page of https://gravity-ui.com/ is responsive from 320px to any size right now.

@LehaRybkoha
Copy link
Contributor Author

No. It was made according to web-design. It will be fixed after integrating responsive version of landing. If you remove horizontal paddings, buttons will be fine and in responsive version paddings will be removed

What is "responsive version of landing"? Every page of https://gravity-ui.com/ is responsive from 320px to any size right now.

A little missunderstanding)
I mean a new version of mobile design, where is no horizontal paddings.
Anyway, I agree with you. Hiding text into buttons now.
Review again pls

@LehaRybkoha LehaRybkoha merged commit 94d67b3 into main Feb 1, 2024
1 check passed
@LehaRybkoha LehaRybkoha deleted the feat/article-buttons branch February 1, 2024 19:18
imsitnikov pushed a commit that referenced this pull request Feb 2, 2024
…prev navigation between articles (#159)"

This reverts commit 94d67b3.
imsitnikov added a commit that referenced this pull request Feb 2, 2024
…prev navigation between articles (#159)" (#168)

This reverts commit 94d67b3.

Co-authored-by: Maksim Sitnikov <[email protected]>
LehaRybkoha pushed a commit that referenced this pull request Feb 2, 2024
…es next/prev navigation between articles (#159)" (#168)"

This reverts commit 1c118e5.
LehaRybkoha added a commit that referenced this pull request Feb 5, 2024
…igation between articles (#169)

* feat: Added ArticleNavigation component, which provides next/prev navigation between articles (#159)

* fix: fixed section links and mdx rendered margin

* fix: a little refactoring

* fix: removed unuseful functions

---------

Co-authored-by: Aleksei <[email protected]>
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.

3 participants