Skip to content

Conversation

@j0hannesr0th
Copy link
Contributor

@j0hannesr0th j0hannesr0th commented Dec 2, 2023

Topic and Scope

@seyfeb fixes #1897

The way I did it gave me the best visual experience on Desktop, Tablet and Smartphone:

2023-12-02_214909.mp4

Concerns/issues

none.

Formal requirements

There are some formal requirements that should be satisfied. Please mark those by checking the corresponding box.

  • I did check that the app can still be opened and does not throw any browser logs
  • I created tests for newly added PHP code (check this if no PHP changes were made)
  • I updated the OpenAPI specs and added an entry to the API changelog (check if API was not modified)
  • I notified the matrix channel if I introduced an API change

@github-actions
Copy link

github-actions bot commented Dec 2, 2023

Test Results

     27 files  1 242 suites   5m 23s ⏱️
   510 tests    510 ✔️ 0 💤 0
4 590 runs  4 589 ✔️ 1 💤 0

Results for commit cf08dd6.

@seyfeb
Copy link
Collaborator

seyfeb commented Dec 3, 2023

Two comments on this (I did not yet look at the code):

  • The related issue was opened by me. Although there was nobody arguing agains this, there was also nobody . Well, except for you obviously ;) Maybe, we can get at least one other opinion from @christianlupus ?
  • Currently there is a large PR on the way which is a preparation of the switch to Vue.js 3. The PR also switched to the new composition API, so I expect the changes here to be incompatible. I'd like to wait with merging any further frontend modification before having merged that one. New features should then be based on the new code. We are currently working out any issues with the PR and I'm optimistic it will be merged soon.

Looking at the video it looks good to me. I like that unchecked ingredients are much more visible so they are overlooked less easily when cooking.

@christianlupus
Copy link
Collaborator

In general, I like the idea. One could think the same technique (at least the dimming) to be used for the instructions as well. I will have a look if I can quickly port it to the new file structure.

@christianlupus
Copy link
Collaborator

As rebasing is hard (used master branch), I recreated a branch and used that for rebasing. Thus, this PR is superseeded by #1910.

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.

Remove checkmarks and replace with strikethrough

3 participants