LUN-357: Replace vocabulary detail modal with own route #306
Conversation
steffenkleinle
requested review from
ztefanie,
sarahsporck and
f1sh1918
as code owners
| @@ -0,0 +1,44 @@ | |||
| import React from 'react' | |||
There was a problem hiding this comment.
Just moved from VocabularyListScreen
| @@ -2,11 +2,11 @@ import React, { ReactElement } from 'react' | |||
| import { widthPercentageToDP as wp } from 'react-native-responsive-screen' | |||
There was a problem hiding this comment.
This is why I don't really like route specific component folders, once it is used in multiple routes it has to be moved/renamed.
There was a problem hiding this comment.
Yes i see, but the need to move and rename a file, just happend for the first time and will maybe occur once in a few weeks/months, whereas I am looking for files every day and finding them in a folder with ~100 files is a really a pain.
There was a problem hiding this comment.
yeah there are different opinions about this which is definitely fine. I personally find it worse that I have to look in multiple folders to find the right file than to find the file in a large folder. No need to change this though, especially if you favor the other way.
There was a problem hiding this comment.
it is now merged. you can base on it
There was a problem hiding this comment.
Already on it 🚀 Assuming your comment is regarding #286 and not this discussion :D
steffenkleinle
force-pushed
the
LUN-357-vocabulary-detail-screen
branch
from
0a4429c to
c4dfe41
Compare
| @@ -2,11 +2,11 @@ import React, { ReactElement } from 'react' | |||
| import { widthPercentageToDP as wp } from 'react-native-responsive-screen' | |||
There was a problem hiding this comment.
Yes i see, but the need to move and rename a file, just happend for the first time and will maybe occur once in a few weeks/months, whereas I am looking for files every day and finding them in a folder with ~100 files is a really a pain.
|
I think also the Feedback modal should be added to both screens. If you do not want / can do it now, please open a ticket, so we do not forget about it. |
Currently not possible since #286 is still open which obviously blocks this. Created https://issues.tuerantuer.org/browse/LUN-358 |
…tail-screen # Conflicts: # src/routes/vocabulary-list/VocabularyListScreen.tsx # src/routes/vocabulary-list/__tests__/VocabularyListScreen.spec.tsx
so the list header is not broken as mentioned here? #307 (comment) |
|
Sorry, did not test this again, just checked for the "finish"-button of the last word. |
…tail-screen # Conflicts: # src/routes/vocabulary-list/VocabularyListScreen.tsx # src/routes/vocabulary-list/components/VocabularyListModal.tsx # src/routes/vocabulary-list/components/__tests__/VocabularyListModal.spec.tsx
I am quite sure this was already broken before, will fix. |
Please check again |
This pull request belongs to an issue on our bugtracker.
You can find it there by looking for an issue with the key which is mentioned in the title of this pull request.
It starts with the keyword LUN.
Done:
VocabularyDetailroute for vocabulary detail (previously: VocabularyDetailModal)VocabularyListto own component (preparation for LUN-132: Favorites)