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

responsive layout #78

Merged
merged 24 commits into from
Sep 27, 2019
Merged

responsive layout #78

merged 24 commits into from
Sep 27, 2019

Conversation

sualko
Copy link
Collaborator

@sualko sualko commented Sep 24, 2019

This is work in progress, but the startscreen and the gallery is already responsive.

fix #22

2019-09-24 19 50 12 localhost 18aa8cbc9815
2019-09-24 19 50 33 localhost 6495004e36c1

@andi34
Copy link
Collaborator

andi34 commented Sep 24, 2019

Great! Thanks a lot for all your work. Going to review your open pull request tomorrow morning.

@sualko
Copy link
Collaborator Author

sualko commented Sep 25, 2019

Do you have an idea why the sidenav style is missing in the sass file?

@andi34
Copy link
Collaborator

andi34 commented Sep 25, 2019

Seems like I forgot to add it there, I always only used the style.css file

@andi34
Copy link
Collaborator

andi34 commented Sep 25, 2019

I am still learning all the stuff here. Learning by doing to be true 😅🤗

@tobiashaas
Copy link
Contributor

Ich bin auch gerade dabei dass alles Responsive umzusetzen! - leider zu langsam gewesen :)
Werde aber ähnlich wie das BlueGrey Theme, einen anderen style bauen ;-)
Könnte man ja dann mit dazu nehmen!

Kann mir noch einer erklären für was genau die Sass Dateien sind? Hab das noch nicht ganz verstanden ...

Liebe Grüße

@andi34
Copy link
Collaborator

andi34 commented Sep 25, 2019

@tobiashaas klar wegen dem Farbtheme. Config kann man entsprechend dann umbauen das man ein Auswahlmenü hat. kann ich übernehmen den Part sobald dein Theme fertig ist.

Bezüglich sass muss ich leider auch passen.

@sualko
Copy link
Collaborator Author

sualko commented Sep 25, 2019

Sass is a superset of css. It adds enclosures, variables, functions, includes and a lot more to css. On the downside it requires a transformation step, because browsers don't support sass. Therefore you should only edit those sass files and never the produced css files.

If you plan to change the style, please wait until this pr is finished, because in order to make the whole thing responsive I have to make some structural changes.

@andi34
Copy link
Collaborator

andi34 commented Sep 25, 2019

Thanks a lot for the explanation and your work!

@sualko
Copy link
Collaborator Author

sualko commented Sep 25, 2019

2019-09-25 14 04 06 localhost 572a8b35771a
2019-09-25 14 03 15 localhost 2d5debbe36fb
2019-09-25 14 03 49 localhost 705a0fa575d1
2019-09-25 14 03 37 localhost c470da7482c3

This pull request needs some really good tests, because I'm not sure if I missed something. Also I have to fix the bluegray theme.

After that I would start to restructure the code to have a foundation for some enhancements regarding the collage.

@andi34
Copy link
Collaborator

andi34 commented Sep 25, 2019

Could the sidenavbar (filter menu) be a little bit transparent?

@andi34
Copy link
Collaborator

andi34 commented Sep 25, 2019

Also as mentioned I guess the video code is still needed, e.g. to use picamera for preview

@tobiashaas
Copy link
Contributor

Ich finde es gut dass die Sidebar nicht transparent ist.
Hatte letztens einen Hintergrund bei dem man kaum lesen konnte wie der Filter heißt 🙈

@andi34
Copy link
Collaborator

andi34 commented Sep 25, 2019

currently i only get a white page

Screenshot_2019-09-25_18-36-57

@sualko
Copy link
Collaborator Author

sualko commented Sep 26, 2019

Did you compile the sass files? If not, move to the resources directory and run npm build or yarn build.

@andi34
Copy link
Collaborator

andi34 commented Sep 26, 2019

got it working now:

npm install -g sass
sass resources/sass/style.scss resources/css/style.css 

What i noticed so far:

  • the print confirmation dialogue is missing now (those parts are missing in the sass file)
  • countdown animation might need adjustment (was smoother before?)
  • QR-Code is missing the text on desktop view, on mobile view still oversized
  • Home-Button and Fork-Me logo conflict on mobile view

Besides the nitpicks it looks great :) Thanks a lot!

@sualko
Copy link
Collaborator Author

sualko commented Sep 27, 2019

Tween lite is now back. Would love to reduce the number of dependencies, but I think here I was a bit fast.

@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

Tween lite is now back. Would love to reduce the number of dependencies, but I think here I was a bit fast.

Thanks :)

I've send you a mail with some information.

@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

Found one issue: show date in caption doesn't work.
We need to remove

.pswp__caption {
    display: none;
}

index.php Outdated Show resolved Hide resolved
@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

IMO ready to merge. It'll be ok?

@sualko
Copy link
Collaborator Author

sualko commented Sep 27, 2019

It'll be ok?

I hope so :-)

@andi34 andi34 merged commit 8c829c4 into andreknieriem:master Sep 27, 2019
@sualko
Copy link
Collaborator Author

sualko commented Sep 27, 2019

Did you rebase this pr? This messed everything up with my refactoring branch...

@sualko
Copy link
Collaborator Author

sualko commented Sep 27, 2019

I think you cherry-picked the whole branch 😢 Please never ever do this again. I have now no idea how I should integrate all my changes from the refactor branch.

@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

Rebased and merged. Makes it easier to go back to a specific commit if something breaks.
Sorry if it made trouble for you. But don't worry, i know git really good and i've no problem to fix it later further if needed.

@sualko use https://github.com/andi34/photobooth/tree/feature/refactor
In case origin is your github clone (it is if you have cloned from your git):

git checkout -b backup
git remote add andi34 https://github.com/andi34/photobooth
git fetch andi34
git checkout andi34/feature/refactor
git push origin HEAD:feature/refactor -f

in case you miss a commit because i haven't seen it:

git log --oneline backup

You'll see the commit message (one line) and the hash you need to cherry-pick it.

@sualko
Copy link
Collaborator Author

sualko commented Sep 27, 2019

Don't worry, I know git really good.

I don't think so. There is absolutely no reason for such an action.

Makes it easier to go back to a specific commit if something breaks

Even after a merge you can go back to every commit you like and in this case even a fast-forward had been possible. Now if you compare different branches or forks, git shows you all commits twice.

@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

From my work on android i know there'll be more trouble compared merge with commits rebased. Also all the "merge xyz" commits which get created imo don't look nice in history.
On rebase and merge the risk of a mismerge is a lot lower.

But i see it's solved now. Sorry again.

Edit:
An example how bad all the "merge" can look like in git history:
Screenshot_20190927-153841

@sualko
Copy link
Collaborator Author

sualko commented Sep 27, 2019

Rebased and merged

If you know git that well, than you know that this is not possible.

all the "merge xyz" commits which get created imo don't look nice in history.

fast-forward? Who is interested in a nice history? Nobody (or use gitk or something else). It's more important that I know that my changes are in a branch. If you rebase something I have to check if you changed my commits, because they will get a new id.

From my work on android

Please look at other open source project and see how they handle merges and thing about why.

@sualko sualko deleted the feature/responsive branch September 27, 2019 13:42
@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

@sualko you're familiar with Gerrit?
I could talk to my friend if it's ok to hijack https://gerrit.unlegacy-android.org/ for the refactoring. This would also make it easier for you to adjust changes if needed.

Edit:
Github gives the possibility to rebase and merge a pull request. If it's going over the Webinterface you can be sure all changes are in.

Screenshot_2019-09-27_15-51-12

@sualko
Copy link
Collaborator Author

sualko commented Sep 27, 2019

I don't know Gerrit, but it was developed for svn and therefore I don't know how helpful this tool is. I can only tell you that I contributed in a lot of open source projects (even large ones like Nextcloud) and I'm the main developer of JSXC which has several thousands of active users.

I understand that you like to have a linear commit history, but I think the disadvantages are to huge (no signing, no comparison, difficult merges, ...). Please look at a fast-forward merge if you like to keep the history flat.

The git branch overview also doesn't look that good:
2019-09-27 15 40 21 github com 98c0b29e17b5

@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

Gerrit is a really useful reviewing tool, especially as long as changes are WIP. You can update changes until they are ready, always return to a previous patchset of the change. It's also possible to view the changes between different patchset of the commit.
A review system allows other devs to vote on made changes and discuss changes if needed.
To submit changes to Gerrit every commit needs a unique Commit-ID which will be added at git commit if it's setup. If a change is cherry-picked to a different project it'll remain the unique Commit ID.
From Gerrit changes can be submitted to e.g. GitHub.

But well, guess we'll forget about that here and concentrate on the important things.
On your next PR I can click on "merge" instead "rebase and merge".

@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

And i am sorry GitHub doesn't show the merge in the network graph. But on top of the PR you see that everything is in.
Screenshot_20190927-162123__01

@thymon13
Copy link
Contributor

Works with Google chrome.

But I use "FullScreenWebBrowser" app to my Android Tablet.
the display has a problem with that : https://youtu.be/PSsM1K6z3hs

@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

You could try opera, from the screenshots on play store it has an option for fullscreen.
Screenshot_20190927-210814

@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

@thymon13
Copy link
Contributor

thymon13 commented Sep 27, 2019

@andi34 Opera is not real fullscreen. I tested
https://play.google.com/store/apps/details?id=de.ozerov.fully --> Same problem

@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

Which webview used?
Screenshot_20190927-213531
Screenshot_20190927-213522

@thymon13
Copy link
Contributor

@andi34 You are the BEST !! Thanks a lot !!

...I'm really not used to android

@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

Changing it fixed it?

@thymon13
Copy link
Contributor

YEEES !! I have updated "webview android system" from PlayStore before.

@andi34
Copy link
Collaborator

andi34 commented Sep 27, 2019

Good to.know, thanks for the feedback 🙂

@thymon13
Copy link
Contributor

I'have found a bug.

I take a picture -> I click on QR code -> I click on screen to back --> I click home : OK
I take a picture -> I click on QR code --> I wait a few minute -> I click on screen to back > I click home : NOK I can't back to Home.

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.

Create responsive design
4 participants