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

Fix problem selecting filter in construction of segments/reports/etc by Enter keydown hitting #7800

Conversation

Noa83
Copy link
Contributor

@Noa83 Noa83 commented Aug 19, 2019

Please be sure you are submitting this against the staging branch.

Q A
Bug fix? Yes
New feature? No
Automated tests included? No
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks? No
Deprecations? No

Description:

This bug is encountered in many cases in whiches selection by keydown is available to select filters (Segments, Reports, Points, etc...).
To illustrate the segment's case example, some screenshots:
When you're searching a filter by typing the first letters of it and then try to select it by Enter hitting, the field does not appear correctly (blank instead of text).

screenshotBug1

If you try to do it once again, the letters you've typed before Enter hitting are now available in the selection.

screenshotBug2

Steps to reproduce the bug:

  1. Create a new segment
  2. Try to add a filter by typing the first letter of the field you want to select and then hit Enter key to select it.

Steps to test this PR:

  1. Load up this PR or Load up on your mautic (assets generate + cache clear needed)
  2. Reproduce the steps above
  3. You can now select filter by keydown hitting.

@npracht npracht added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test labels Aug 19, 2019
@npracht npracht added this to the 2.16.0 milestone Aug 19, 2019
@npracht npracht added the T1 Low difficulty to fix (issue) or test (PR) label Aug 19, 2019
@npracht npracht added this to Ready to Test (first time) in Mautic 2 Aug 19, 2019
RCheesley
RCheesley previously approved these changes Aug 23, 2019
Copy link
Sponsor Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Reproduced & tested patch, works perfectly - thanks for fixing this annoying bug!

@npracht npracht moved this from Ready to Test (first time) to Ready to Test (confirmation) in Mautic 2 Aug 26, 2019
@npracht npracht moved this from Ready to Test (confirmation) to Ready to Test (first time) in Mautic 2 Aug 26, 2019
@RCheesley RCheesley added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test labels Aug 29, 2019
@escopecz escopecz moved this from Ready to Test (first time) to Ready to Test (confirmation) in Mautic 2 Sep 2, 2019
Mautic 2 automation moved this from Ready to Test (confirmation) to Changes Requested / Review Sep 3, 2019
@npracht
Copy link
Member

npracht commented Sep 3, 2019

@RCheesley we just added a new commit to fix tag creation on form action and csv import.

How to test

Before installing the PR, test the 2 issues:

  1. Create a form with action "add tag", try to add a new tag and see issue.
  2. Import a csv file of contacts. Add a tag (a new one) and see issue.

Install PR and reproduce. 🎉

Mautic 2 automation moved this from Changes Requested / Review to Ready to Test (first time) Sep 3, 2019
Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

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

👍 works for me

@kuzmany kuzmany added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Sep 30, 2019
@npracht npracht moved this from Ready to Test (first time) to Ready to Commit (passed testing) in Mautic 2 Oct 3, 2019
@npracht
Copy link
Member

npracht commented Jan 17, 2020

While testing the whole 3.0.0-alpha app, I noticed this issue is still existing. I think we should consider adding this PR in 3.x branch.

@escopecz
Copy link
Sponsor Member

If there will be another M2.x release then we can merge it to staging and then merge staging to the 3.x branch. If we won't find a release leader for another M2.x release then we can merge this directly to the 3.x branch.

@npracht
Copy link
Member

npracht commented Jan 17, 2020

I believe with the planning given by Alan that we won't have another 2.x release before 3.0.0 is out !

@escopecz
Copy link
Sponsor Member

That may be true. Especially when no one wants to be a release leader for it :)

Another question is if releasing M3.0.0 will automatically sunset M2.x.x. I don't think so as we have to give time to Mautic users to prepare for the migration. But that depends on what the community agrees on and if there is someone willing to support M2 for several more months. The two of us won't figure that out in this thread.

@kuzmany
Copy link
Member

kuzmany commented Jan 20, 2020

There are not human resources for maintenance 2.x.x, then we should focus on 3.x.x.
All current PRs we can step by step rework for 3.x.x if need it.
Or we can release 3.0.0 as public preview for agencies, but 3.1.0 as stable release with bugfixes.
Release leader is time-consuming job. Companies whose earn money from Mautic, could sponsor some activities. That's not possible do it just for fun

Sorry for off-topic, maybe we can move discuss it to forum or slack :)

@npracht
Copy link
Member

npracht commented Jan 20, 2020

Hi @Noa83 can you check if your PR is mergeable in https://github.com/mautic/mautic/tree/3.x and manage conflicts if needed?
Then we give a chance to the PR to be merged in 3.0.0. Thanks !

@npracht npracht modified the milestones: 2.16.0, 2.15.4 Jan 23, 2020
@RCheesley
Copy link
Sponsor Member

@npracht I noticed this has me assigned as a reviewer, do you still require me to test?

@npracht
Copy link
Member

npracht commented Jan 24, 2020

Nope ! Already ready to commit :)

@npracht npracht removed the request for review from RCheesley January 24, 2020 10:23
@dennisameling dennisameling merged commit 375310d into mautic:staging Jan 29, 2020
Mautic 2 automation moved this from Ready to Commit (passed testing) to Merged Jan 29, 2020
@dennisameling dennisameling removed the ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged label Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs T1 Low difficulty to fix (issue) or test (PR)
Projects
No open projects
Mautic 2
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

7 participants