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

Add gmf-mobile-nav-back-on-click directive #1446

Merged
merged 1 commit into from
Jun 22, 2016

Conversation

fredj
Copy link
Member

@fredj fredj commented Jun 22, 2016

And use it to close the gmf-themeselector on mobile.

Fixes #1344

And use it to close the gmf-themeselector on mobile.
* gmf-mobile-nav-back-on-click
* gmf-themeselector-currenttheme="mainCtrl.theme">
* </gmf-themeselector>
*
Copy link
Member

@sbrunner sbrunner Jun 22, 2016

Choose a reason for hiding this comment

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

No htmlAttributes?

Copy link
Member Author

Choose a reason for hiding this comment

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

This directive has no html attributes

Copy link
Member

Choose a reason for hiding this comment

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

Sorry I just see the attributes from the theme selector, add ..., forget my comment :-)

@pgiraud
Copy link
Contributor

pgiraud commented Jun 22, 2016

This is a pretty elegant workaround.
+1

@fredj
Copy link
Member Author

fredj commented Jun 22, 2016

Thanks for the review

@fredj fredj merged commit 5aa7cde into camptocamp:master Jun 22, 2016
@fredj fredj deleted the back_on_theme_select_alt branch June 22, 2016 11:39
@pgiraud
Copy link
Contributor

pgiraud commented Jun 22, 2016

@fredj did you check if there are still cases where we want to keep the previous gmf-mobile-nav-back directive?

@fredj
Copy link
Member Author

fredj commented Jun 22, 2016

yes, gmf-mobile-nav-back is not used anymore, do you think we should remove it ?

@pgiraud
Copy link
Contributor

pgiraud commented Jun 22, 2016

If it's not used, I'm in favour to remove it. It adds unrequired complexity. This "back-click" directive is much more easy to use and understand and is useful in many contexts.

@fredj
Copy link
Member Author

fredj commented Jun 22, 2016

ok, and the code is in the git history if needed

@fredj
Copy link
Member Author

fredj commented Jun 22, 2016

I will create a PR

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.

Mobile: selecting currently selected theme doesn't close themes panel
3 participants