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.

Sign up for GitHub

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

Jump to bottom

Align Identity card with dapps explorer #1561

Merged
merged 2 commits into from
May 8, 2023
Merged

Align Identity card with dapps explorer #1561

merged 2 commits into from
May 8, 2023

Conversation

nmattia
Copy link
Collaborator

@nmattia nmattia commented May 8, 2023

The "Internet Identity" image card uses negative borders to make up for its container's padding; however the wrong padding values were used. This makes sure the correct margins are used to ensure the cards are aligned on the management page.

Before:

Screenshot 2023-05-08 at 10 28 15

After:

Screenshot 2023-05-08 at 10 28 25

nmattia and others added 2 commits May 8, 2023 10:28
@nmattia
Align Identity card with dapps explorer
eb6d00e 
The "Internet Identity" image card uses negative borders to make up for
its container's padding; however the wrong padding values were used.
This makes sure the correct margins are used to ensure the cards are
aligned on the management page.
@github-actions
🤖 Selenium screenshots auto-update
a7ea270
@nmattia nmattia requested review from peterpeterparker and frederikrothenberger and removed request for peterpeterparker May 8, 2023 09:16
frederikrothenberger
frederikrothenberger approved these changes May 8, 2023
View reviewed changes
Copy link
Member

@frederikrothenberger frederikrothenberger left a comment

Choose a reason for hiding this comment

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

Unrelated, but the mobile screenshots are off. Is this to do with min-width exceeding the mobile screen width? We should probably switch to a more realistic mobile device resolution...

@nmattia nmattia added this pull request to the merge queue May 8, 2023
@nmattia
Copy link
Collaborator Author

nmattia commented May 8, 2023

Unrelated, but the mobile screenshots are off. Is this to do with min-width exceeding the mobile screen width? We should probably switch to a more realistic mobile device resolution...

Yeah I know, spotted this here too. I'll have a look into this but I suspect it's related to last-minute changes we made last week to fix the container width on desktop. I think the resolution is somewhat realistic (iPhone SE IIRC)

@frederikrothenberger
Copy link
Member

Unrelated, but the mobile screenshots are off. Is this to do with min-width exceeding the mobile screen width? We should probably switch to a more realistic mobile device resolution...

Yeah I know, spotted this here too. I'll have a look into this but I suspect it's related to last-minute changes we made last week to fix the container width on desktop. I think the resolution is somewhat realistic (iPhone SE IIRC)

But is it actually broken on an iPhone SE now?

@nmattia
Copy link
Collaborator Author

nmattia commented May 8, 2023

But is it actually broken on an iPhone SE now?

According to Chrome's mobile emulator, it is not; might be because of different pixel densities. Will need to look into it to get a better idea of what's happening.

Merged via the queue into main with commit 0950e55 May 8, 2023
@nmattia nmattia deleted the nm-align branch May 8, 2023 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frederikrothenberger frederikrothenberger frederikrothenberger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nmattia @frederikrothenberger