Skip to content

Comments

Fullscreen improvement#86

Open
adaniello wants to merge 1 commit intoalfredobarron:masterfrom
adaniello:fullscreen-improvement
Open

Fullscreen improvement#86
adaniello wants to merge 1 commit intoalfredobarron:masterfrom
adaniello:fullscreen-improvement

Conversation

@adaniello
Copy link
Contributor

A simple optimization (in my opinion): i removed anchor tag from fullscreen button definition and event listeners.

In this commit i want to:

  1. avoid nested anchors (see example in http://alfredobarron.github.io/smoke/#/fullscreen);
  2. have a cleaner code that works fine in all circumstances (have you tried to place it in a navbar?);
  3. feel free to use my html code for button;
  4. support also multiple button fullscreen in the same page;
  5. optimize code removing event listener.

A working example at http://codepen.io/anon/pen/gpZRar

A simple optimization (in my opinion): i removed anchor tag from fullscreen button definition and event listener.
@alfredobarron
Copy link
Owner

1 Nested anchors i use for the example
2 Yes i use in a navbar and insert the anchor into a div
5 The listeners are for keyboard event scape.

@adaniello
Copy link
Contributor Author

@alfredobarron ok for listeners, now i understand. I'll add the listners.

About the "lite" code/template (removing anchor tag), i think that this is better because user is free to use his code for button: user can choose about style, position, parent with fewer restrictions.
And then, when i tell you about navbar, i tell about a list item in bootstrap style.
What do you think about? Can we try a new way or not?
Thanks

P.S.: please, update example in doc removing anchor :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants