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

Added a page for downloading mobile application #688

Merged
merged 2 commits into from
Nov 24, 2020
Merged

Added a page for downloading mobile application #688

merged 2 commits into from
Nov 24, 2020

Conversation

M4yankChoudhary
Copy link
Contributor

@M4yankChoudhary M4yankChoudhary commented Oct 3, 2020 •
edited
Loading

Added a download page pointing to F-Droid and Play Store.

Glandos
Glandos requested changes Oct 31, 2020
View reviewed changes
Copy link
Member

@Glandos Glandos left a comment

Choose a reason for hiding this comment

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

Very nice! I introduced some changes that I found to be better before merging.
Can you review them and if possible, apply them?

ihatemoney/templates/layout.html Outdated Show resolved Hide resolved
ihatemoney/static/css/download_mobile_app.css Outdated Show resolved Hide resolved
ihatemoney/static/css/download_mobile_app.css Outdated Show resolved Hide resolved
ihatemoney/templates/download_mobile_app.html Outdated Show resolved Hide resolved
@M4yankChoudhary
Copy link
Contributor Author

M4yankChoudhary commented Nov 1, 2020 •
edited
Loading

Sure, I will do that.

M4yankChoudhary
M4yankChoudhary commented Nov 1, 2020
View reviewed changes
Copy link
Contributor Author

@M4yankChoudhary M4yankChoudhary left a comment

Choose a reason for hiding this comment

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

Resolved

M4yankChoudhary
M4yankChoudhary commented Nov 1, 2020
View reviewed changes
Copy link
Contributor Author

@M4yankChoudhary M4yankChoudhary left a comment

Choose a reason for hiding this comment

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

Added Bootsrap class for color.

@M4yankChoudhary
Copy link
Contributor Author

Added apple-store option for downloading the app.

@Glandos
Copy link
Member

Glandos commented Nov 4, 2020

I made a small modification in the CSS, because I provided the wrong patch :)

I also tried to format web.py, without success.

Is there any reason to include both PNG and SVG for Apple, or is it a simple mistake?

@M4yankChoudhary
Copy link
Contributor Author

M4yankChoudhary commented Nov 5, 2020 •
edited
Loading

I made a small modification in the CSS, because I provided the wrong patch :)

I also tried to format web.py, without success.

Is there any reason to include both PNG and SVG for Apple, or is it a simple mistake?

Yes that's my mistake, sorry i forgot to delete the png file. I will delete it.

indatwood
indatwood reviewed Nov 8, 2020
View reviewed changes
ihatemoney/web.py Outdated Show resolved Hide resolved
indatwood
indatwood reviewed Nov 8, 2020
View reviewed changes
ihatemoney/web.py Outdated Show resolved Hide resolved
indatwood
indatwood reviewed Nov 8, 2020
View reviewed changes
ihatemoney/web.py Outdated Show resolved Hide resolved
@indatwood indatwood mentioned this pull request Nov 8, 2020
Closed
@Glandos Glandos mentioned this pull request Nov 8, 2020
Merged
@M4yankChoudhary
Copy link
Contributor Author

M4yankChoudhary commented Nov 9, 2020 •
edited
Loading

I tried to delete the png file but it's not deleting. I think only the maintainer can delete this file.
Screenshot_20201109_152550

@Glandos
Copy link
Member

Glandos commented Nov 13, 2020

@M4yankChoudhary if you use the GitHub UI, you have to go to https://github.com/M4yankChoudhary/ihatemoney/blob/issue597/ihatemoney/static/images/app-store.png and click on the trash icon at the top right.
It seems that I can do it myself, since you provided write access to your branch, but I'll let you do it if you prefer :)

@indatwood
Copy link
Contributor

Thanks for the update :-) It seems almost all good for me, once comments by @Glandos will be addressed, thanks!

@Glandos
Copy link
Member

Glandos commented Nov 21, 2020

So now… GitHub doesn't want to merge because of conflicts. And it seems that some commits are weird branches.

Anyway, you can squash all commits by doing git reset --soft b89bd1 (the parent commit of your first) in your branch, then commit with a new message, and finally force-push.

Before committing, can you try to execute make flake8 black to be sure formatting is OK?

@M4yankChoudhary
Copy link
Contributor Author

M4yankChoudhary commented Nov 23, 2020 •
edited
Loading

Before committing, can you try to execute make flake8 black to be sure formatting is OK?

@Glandos
I tried to run the make flake8 black command after installing the flake8 using pip but it says:

make: *** No rule to make target 'flake8'. Stop.

@Glandos Glandos merged commit 6ec7dd6 into spiral-project:master Nov 24, 2020
@Glandos
Copy link
Member

Glandos commented Nov 24, 2020

I made another mistake: it was make black isort. I merged your request like this and format it in #699 so everything is fine.

Thanks for your contribution, it is really appreciated.

TomRoussel pushed a commit to TomRoussel/ihatemoney that referenced this pull request Mar 2, 2024
@M4yankChoudhary @Glandos
Added a page for downloading mobile application (spiral-project#688)
adbdd77 
Fixes spiral-project#597 
Fixes spiral-project#697 

Co-authored-by: Glandos <bugs-github@antipoul.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hustenbonbon Hustenbonbon Hustenbonbon left review comments

@Glandos Glandos Glandos approved these changes

@indatwood indatwood Awaiting requested review from indatwood

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

Successfully merging this pull request may close these issues.
4 participants
@M4yankChoudhary @Glandos @indatwood @Hustenbonbon