vikunja
/
frontend
Archived
6
39
Fork 49
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

fix: remove mentioning of context #1017

Merged
konrad merged 3 commits from dpschen/frontend:feature/fix-translation-improvement into main 2021-11-22 21:36:18 +00:00
Conversation 5 Commits 3 Files Changed 7 +47 -15
dpschen commented 2021-11-16 13:17:32 +00:00
Member
Copy Link

All other buttons on the task detail page also don't mention "this task" since that context is already given by the page you are on

All other buttons on the task detail page also don't mention "this task" since that context is already given by the page you are on
frederick commented 2021-11-16 13:34:27 +00:00
Member
Copy Link

Hi dpschen!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://1017-featurefix-translation-improvement--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.

Have a nice day!

Beep boop, I'm a bot.

Hi dpschen! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://1017-featurefix-translation-improvement--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/. Have a nice day! > Beep boop, I'm a bot.
konrad requested changes 2021-11-16 19:59:47 +00:00
konrad left a comment
Owner

Looks like the test is looking for "Assign this task to a user" - could you change it there to make it pass again?

Looks like the test is looking for "Assign this task to a user" - could you change it there to make it pass again?
dpschen commented 2021-11-17 11:15:27 +00:00
Author
Member
Copy Link

Hahaha, I restarted the tests a few times and was really confused… explains a lot :D :D

Hahaha, I restarted the tests a few times and was really confused… explains a lot :D :D
dpschen force-pushed feature/fix-translation-improvement from 44da9454cb to 4968d07d0e 2021-11-17 13:17:24 +00:00 Compare
dpschen commented 2021-11-17 13:20:27 +00:00
Author
Member
Copy Link

Fixed. I had to downgrade to cypress 8. See: #1020

Also I added a directive that allows to select elements via a data attribute.
I used a directive so that the data attributes are only added when needed.

Fixed. I had to downgrade to cypress 8. See: https://kolaente.dev/vikunja/frontend/issues/1020 Also I added a directive that allows to select elements via a data attribute. I used a directive so that the data attributes are only added when needed.
👍 1
dpschen reviewed 2021-11-17 13:22:02 +00:00
src/views/tasks/TaskDetailView.vue Outdated
@ -273,2 +273,3 @@
type="secondary"
v-shortcut="'a'">
v-shortcut="'a'"
v-cy="'taskDetail.assign'"
dpschen commented 2021-11-17 13:22:02 +00:00
Author
Member

This directive gets expanded to data-cy="taskDetail.assign"
My idea was to use this pattern:

COMPONENT_ID.LOCAL_SELECTOR_ID
This directive gets expanded to `data-cy="taskDetail.assign"` My idea was to use this pattern: ``` COMPONENT_ID.LOCAL_SELECTOR_ID ```
👍 1
dpschen marked this conversation as resolved
dpschen commented 2021-11-17 14:09:44 +00:00
Author
Member
Copy Link

@konrad Can you help me with the sed? Locally this worked for me

@konrad Can you help me with the `sed`? Locally this worked for me
konrad reviewed 2021-11-17 19:35:38 +00:00
src/directives/cypress.ts Outdated
@ -0,0 +9,4 @@
const cypressDirective: Directive = {
mounted(el, {value}) {
if (
(window.CYPRESS_DATA_ATTRIBUTES === true || import.meta.env.DEV) &&
konrad commented 2021-11-17 19:35:38 +00:00
Owner

What about doing this with an env variable instead of another window.?

What about doing this with an env variable instead of another `window.`?
dpschen commented 2021-11-17 19:40:54 +00:00
Author
Member

I wanted to first. But then we would need to build it just for the tests again. Because the original idea of the directive is that there are no data-cy attributes in the final build when not needed.

I wanted to first. But then we would need to build it just for the tests again. Because the original idea of the directive is that there are no `data-cy` attributes in the final build when not needed.
konrad commented 2021-11-17 19:51:35 +00:00
Owner

Or maybe just using window.Cypress?

docs (it's not really documented this exists but it should work nevertheless)

Or maybe just using `window.Cypress`? [docs](https://docs.cypress.io/api/commands/window#Start-tests-when-app-is-ready) (it's not really documented this exists but it should work nevertheless)
dpschen commented 2021-11-17 21:06:07 +00:00
Author
Member

I like that. Makes it much simpler =)

I like that. Makes it much simpler =)
dpschen commented 2021-11-18 18:45:22 +00:00
Author
Member

Fixed

Fixed
dpschen commented 2021-11-19 10:30:34 +00:00
Author
Member

Was wrong. Doesn seem to work yet :/
I think window.cypress might be undefined in the moment when the directive is mounted. Then it won't ever be added :/

Was wrong. Doesn seem to work yet :/ I think `window.cypress` might be undefined in the moment when the directive is mounted. Then it won't ever be added :/
konrad commented 2021-11-22 18:28:54 +00:00
Owner

Bummer :/ I guess that's why it wasn't really documented.

Bummer :/ I guess that's why it wasn't really documented.
konrad commented 2021-11-22 18:30:06 +00:00
Owner

Did you check if it is indeed not available when the directive is mounted? (Maybe with a console.log(window.Cypress))

Did you check if it is indeed not available when the directive is mounted? (Maybe with a `console.log(window.Cypress)`)
dpschen commented 2021-11-22 20:32:00 +00:00
Author
Member

That seems to be it.

That seems to be it.
dpschen marked this conversation as resolved
dpschen force-pushed feature/fix-translation-improvement from 4968d07d0e to 55f6af0214 2021-11-18 18:44:32 +00:00 Compare
konrad reviewed 2021-11-22 18:30:44 +00:00
src/directives/cypress.ts Outdated
@ -0,0 +9,4 @@
const cypressDirective: Directive = {
mounted(el, {value}) {
if (
(window.cypress || import.meta.env.DEV) &&
konrad commented 2021-11-22 18:30:44 +00:00
Owner

I think that should be window.Cypress with a capital C.

I think that should be `window.Cypress` with a capital `C`.
👍 1 🎉 1
dpschen commented 2021-11-22 20:32:27 +00:00
Author
Member

Should have seen that…

Should have seen that…
dpschen marked this conversation as resolved
dpschen force-pushed feature/fix-translation-improvement from 55f6af0214 to a0abb3469e 2021-11-22 20:30:43 +00:00 Compare
konrad commented 2021-11-22 20:38:30 +00:00
Owner
Copy Link

Please resolve the conflicts

Please resolve the conflicts
👍 1
konrad approved these changes 2021-11-22 20:43:44 +00:00
konrad left a comment
Owner

Feel free to merge once conflicts are resolved.

Feel free to merge once conflicts are resolved.
dpschen force-pushed feature/fix-translation-improvement from a0abb3469e to 199768f433 2021-11-22 21:08:19 +00:00 Compare
konrad merged commit 981babd691 into main 2021-11-22 21:36:18 +00:00
konrad deleted branch feature/fix-translation-improvement 2021-11-22 21:36:18 +00:00
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
konrad
Labels
Clear labels   
area/internal-code

   
changes requested

   
confirmed

   
dependencies

   
duplicate

This issue or pull request already exists

  
good first issue

   
help wanted

Need some help

  
hosting

   
invalid

Something is wrong

  
kind/bug

Something is not working

  
kind/feature

New feature

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/frontend#1017
No description provided.
Delete Branch "dpschen/frontend:feature/fix-translation-improvement"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?