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(frontend): changed plex, request, and cog buttons to align proper… #928

Merged
merged 3 commits into from
Feb 16, 2021

Conversation

OwsleyJr
Copy link
Collaborator

@OwsleyJr OwsleyJr commented Feb 14, 2021

…ly on smaller mobile UIs

Description

Play on plex/watch trailer, request, and settings cog buttons would break on a smaller mobile device such as an iPhone X. Also aligns media status with the poster.

Screenshot (if UI related)

Before:
before
After:
new version

Todos

  • Sucessfully builds yarn build
  • Translation Keys yarn i18n:extract
  • Database migration created (if required)

@OwsleyJr OwsleyJr force-pushed the bugfix/movie/tv-details-page-alignment-mobile branch from 0431f7a to 3f41760 Compare February 14, 2021 15:52
@OwsleyJr OwsleyJr force-pushed the bugfix/movie/tv-details-page-alignment-mobile branch from 8841903 to 59611a2 Compare February 15, 2021 22:33
@TheCatLady
Copy link
Collaborator

What is the reasoning behind the changes in 59611a2? There are other elements that would also need to be updated as a result of those new changes.

@sct
Copy link
Owner

sct commented Feb 16, 2021

What is the reasoning behind the changes in 59611a2? There are other elements that would also need to be updated as a result of those new changes.

I asked him to change it to this because I didn't like the buttons going to the edge of the screen. They should respect the same margins the rest of the site has. What other components would need to change?

@TheCatLady
Copy link
Collaborator

What is the reasoning behind the changes in 59611a2? There are other elements that would also need to be updated as a result of those new changes.

I asked him to change it to this because I didn't like the buttons going to the edge of the screen. They should respect the same margins the rest of the site has. What other components would need to change?

Ah I see. We also need to apply the same margins from the "Play on Plex" button to the div surrounding the request button and the cog button, right? Otherwise we'll run into the same issue depending if the elements fit on one line or not?

@OwsleyJr
Copy link
Collaborator Author

What is the reasoning behind the changes in 59611a2? There are other elements that would also need to be updated as a result of those new changes.

I asked him to change it to this because I didn't like the buttons going to the edge of the screen. They should respect the same margins the rest of the site has. What other components would need to change?

Ah I see. We also need to apply the same margins from the "Play on Plex" button to the div surrounding the request button and the cog button, right? Otherwise we'll run into the same issue depending if the elements fit on one line or not?

I changed the margin on the div wrapping the three buttons so they will stay the same on one line.

@TheCatLady
Copy link
Collaborator

I changed the margin on the div wrapping the three buttons so they will stay the same on one line.

I just pulled your changes to get a better look. I see what you did now. LGTM!

@sct sct enabled auto-merge (squash) February 16, 2021 07:16
@sct sct merged commit f1c3358 into develop Feb 16, 2021
@sct sct deleted the bugfix/movie/tv-details-page-alignment-mobile branch February 16, 2021 07:16
@github-actions
Copy link

🎉 This PR is included in version 1.20.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants