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

Popover update #155

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Popover update #155

wants to merge 5 commits into from

Conversation

ekongobie
Copy link
Contributor

@ekongobie ekongobie commented Apr 7, 2020

Fixes #151

Make popover scrollable, displayed on click, and hidden on click anywhere else.

ekongobie and others added 2 commits April 7, 2020 12:42
Make popover scrollable, displayed on click, and hidden on click anywhere else.
@rtgdk
Copy link
Collaborator

rtgdk commented Apr 8, 2020

@ekongobie Good. i found one more issue in this - When the input box is selected (which is by default) and then we click on the popover button, it doesn't show the pop over. Only when clicked again, the pop over appears. Can you look into it? [ TO test -> click on a popover, then click on the input box, then again on the same popover]

Also, I just noticed that we have manually added the content in each popover. Changing this pop over would require extra efforts. Can you look into this and propose a new way to load this content? Let me know if you need more info/help regarding this.

Also, I noticed that we manually

@ekongobie
Copy link
Contributor Author

@rtgdk I am looking into the "click" issue.

The default trigger for the popovers is the "click" action, so I can remove that option completely.
Concerning the popover content, we can definitely read that from a text file.
On it!

@ekongobie
Copy link
Contributor Author

@rtgdk I have solved the popover click issue. However, inorder to solve this, the dismissal of the popover is only done when we click "outside", that is, not on the popover icon. Please test for convenience. I also removed the help text from the javascript and put is in a python file; and then I passed the help text to javascript through form field attributes.

Copy link
Collaborator

@rtgdk rtgdk left a comment

Choose a reason for hiding this comment

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

@ekongobie Changes look good. Python looks less cluttered than js code, thanks for that! Left some comments before I merge.

src/app/static/js/editor/codemirror.js Show resolved Hide resolved
src/app/constants.py Show resolved Hide resolved
src/app/templates/app/submit_new_license_namespace.html Outdated Show resolved Hide resolved
@@ -593,6 +590,7 @@
});
}
</script>
<script type="text/javascript" src="{% static 'js/editor/codemirror.js' %}"></script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need this script?

@goneall
Copy link
Member

goneall commented Jun 15, 2021

@ekongobie If you could resolve the merge conflicts, I'll go ahead and merge this 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.

Innovative popover while submitting license
3 participants