Skip to content

Conversation

platon-rov
Copy link
Contributor

@platon-rov platon-rov commented Nov 16, 2021

Related Issue

Part of SAP/fundamental-ngx#7172

Description

Pagination components styles update due to changed specifications.

Screenshots

Please check the pagination component examples. There are too many changes to place here.

https://deploy-preview-2912--fundamental-styles.netlify.app/?path=/docs/components-pagination--first-page

Breaking changes

https://github.com/SAP/fundamental-styles/wiki/Breaking-Changes#pagination-2912

@platon-rov platon-rov requested review from a team November 16, 2021 15:35
@platon-rov platon-rov self-assigned this Nov 16, 2021
@netlify
Copy link

netlify bot commented Nov 16, 2021

✔️ Deploy Preview for fundamental-styles ready!

🔨 Explore the source changes: 2c37bde

🔍 Inspect the deploy log: https://app.netlify.com/sites/fundamental-styles/deploys/619cb1c46a838c00078afc15

😎 Browse the preview: https://deploy-preview-2912--fundamental-styles.netlify.app

@platon-rov platon-rov force-pushed the fix/pagination-rework branch 2 times, most recently from 6c10348 to 92061b1 Compare November 17, 2021 15:21
@InnaAtanasova
Copy link
Contributor

Screen Shot 2021-11-17 at 11 47 56 AM

Screen Shot 2021-11-17 at 11 48 06 AM

Why is the width of the input different in these cases?

@InnaAtanasova
Copy link
Contributor

Screen Shot 2021-11-17 at 11 52 24 AM

The space between 2 and the ... is different from the space between the ... and 500, aka the dots are not in the middle of the page numbers.

@InnaAtanasova
Copy link
Contributor

Why does the input have such big width?
Screen Shot 2021-11-17 at 11 54 44 AM

I think you need to use: width: auto, not width:100%. This will result in this
Screen Shot 2021-11-17 at 11 54 29 AM
:

@InnaAtanasova
Copy link
Contributor

You have Cozy and Mobile modes but the specs says Desktop and Tablet are the same.
Fundamentals:
Screen Shot 2021-11-17 at 11 58 08 AM

Specs:
Screen Shot 2021-11-17 at 11 58 16 AM

@InnaAtanasova
Copy link
Contributor

Screen Shot 2021-11-17 at 12 25 56 PM

Please refactor this. The user should be able to copy the markup in the code section.

@InnaAtanasova
Copy link
Contributor

Screen Shot 2021-11-17 at 12 28 41 PM

You are not supposed to have compact buttons in mobile mode.

@droshev
Copy link
Contributor

droshev commented Nov 18, 2021

@platon-rov can you maintain the scope too?

@platon-rov
Copy link
Contributor Author

platon-rov commented Nov 18, 2021

@InnaAtanasova

#2912 (comment)

Why is the width of the input different in these cases?

#2912 (comment)

Why does the input have such big width? I think you need to use: width: auto, not width:100%. This will result in this

I partially fixed it by setting width: auto as you recommended, thank you. But it's also depends on min\max attribute of the input, for example if we have 500 (three digits) pages then input will be wider then if we have 5 (one digit).

#2912 (comment)

The space between 2 and the ... is different from the space between the ... and 500, aka the dots are not in the middle of the page numbers.

Link always has 9px of horizontal padding, but since content 500 is longer than 2 we have buttons of the different width. So we're seeing illusion that ... is not centered.

#2912 (comment)

You have Cozy and Mobile modes but the specs says Desktop and Tablet are the same.

But below in the table described to use Cozy for tablets.

Screenshot 2021-11-18 at 15 26 40

@platon-rov
Copy link
Contributor Author

@platon-rov can you maintain the scope too?

@droshev What do you mean, change it to feat?

@InnaAtanasova
Copy link
Contributor

@platon-rov can you maintain the scope too?

@droshev What do you mean, change it to feat?

nope, he means you are missing the (styles) part in the PR title. You have: fix: pagination styles update and it should be fix(styles): pagination styles update. I updated it :)

@InnaAtanasova InnaAtanasova changed the title fix: pagination styles update fix(styles): pagination styles update Nov 19, 2021
@InnaAtanasova InnaAtanasova added this to the Sprint 75 - Burlington milestone Nov 19, 2021
Copy link
Contributor

@InnaAtanasova InnaAtanasova left a comment

Choose a reason for hiding this comment

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

Screen Shot 2021-11-19 at 2 09 18 PM

The input follows the specs for Input Field and I couldn't find anything about max-width, only for mix-width, which means the input should grow to fit the content?

@github-actions
Copy link

This pull request is stale because it has been open 2 days with no activity. Remove stale label or comment or this will be closed in 3 days

@github-actions github-actions bot added the stale label Nov 22, 2021
@platon-rov
Copy link
Contributor Author

#2912 (review)

The input follows the specs for Input Field and I couldn't find anything about max-width, only for mix-width, which means the input should grow to fit the content?

min-width: 2.75rem; width: auto accordingly to the Fiori 3 specs.

@github-actions github-actions bot removed the stale label Nov 23, 2021
@platon-rov platon-rov force-pushed the fix/pagination-rework branch from a2a6243 to 2c37bde Compare November 23, 2021 09:17
@platon-rov platon-rov merged commit b21f314 into main Nov 23, 2021
@platon-rov platon-rov deleted the fix/pagination-rework branch November 23, 2021 11:59
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