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 triggers select in Add Goal form #8267

Merged
merged 2 commits into from Jul 6, 2015
Merged

Fix triggers select in Add Goal form #8267

merged 2 commits into from Jul 6, 2015

Conversation

barbushin
Copy link
Contributor

Closes #8244

@tsteur
Copy link
Member

tsteur commented Jul 2, 2015

I still have to click on the select for the changes to be applied, even after I changed the value.

Just had a quick look and it might be enough to listen to the change instead of click event here:

https://github.com/barbushin/piwik/blob/8244_goals_form_fix/plugins/Goals/javascripts/goalsForm.js#L108

That might be the only change needed but not sure re side effects. If that works, we could undo the other changes probably.

@barbushin
Copy link
Contributor Author

Changed .click to .change, but for me it works the same. Like this http://i.imgur.com/gKzOzt0.gif

@barbushin barbushin added the Needs Review PRs that need a code review label Jul 3, 2015
@tsteur
Copy link
Member

tsteur commented Jul 5, 2015

Which browser are you using? It did not work for me on Chrome 43 with MacOSX.

@mattab the select box is now moved to the right see: http://i.imgur.com/gKzOzt0.gif What's your thought on that?

@barbushin
Copy link
Contributor Author

I use Chrome 43, Firefox 34 and Opera 12 on Win8.1x64.

Regarding new select box position, I moved it to the right side because I thought it's HTML bug that it was located in left side. Also I think it will be better if we set vertical-align: top for blocks titles, like this http://i.imgur.com/yV4VIPb.png (current version: http://i.imgur.com/uoytH0r.png).

But anyway, it's up to you, guys, to chose the best solution. I'm not a designer :)

mattab pushed a commit that referenced this pull request Jul 6, 2015
Fix triggers select in Add Goal form
@mattab mattab merged commit b6da1c0 into matomo-org:master Jul 6, 2015
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Jul 6, 2015
@mattab
Copy link
Member

mattab commented Jul 6, 2015

I'm not a designer :)

we're also not designers, but we do our best to build UI that look good and are usable. it's very important part of building a successful and amazing product.

@barbushin
missing space

Could you add some space below this SELECT box as you can see it's too close to the radio boxes below

@barbushin
Copy link
Contributor Author

@mattab What browser do you use? In Chrome 43 and Firefox 34 there is a space already
image

@tsteur
Copy link
Member

tsteur commented Jul 6, 2015

Also I think it will be better if we set vertical-align: top for blocks titles

I like it much better, maybe you can issue another PR?

@barbushin
Copy link
Contributor Author

@tsteur Sure! Just added that fix in #8279

@mattab
Copy link
Member

mattab commented Jul 7, 2015

@mattab What browser do you use? In Chrome 43 and Firefox 34 there is a space already

@barbushin I can see this behavior on both latest Chromium and Firefox on ubuntu

@barbushin
Copy link
Contributor Author

@mattab It's very strange because I can't reproduce it. There is <p> tag with margin-top: 10px for list of radio inputs
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants