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

Disable Fomantic's CSS tooltips #16974

Merged
merged 2 commits into from
Sep 8, 2021
Merged

Conversation

silverwind
Copy link
Member

CSS-only tooltips suffer various issues with positioning and there was only one single instance of them in the templates. Replace that instance with a regular popup and exclude these data-tooltip styles from the Fomantic build.

CSS-only tooltips suffer various issues with positioning and there was
only one single instance of them in the templates. Replace that instance
with a regular popup and exclude these `data-tooltip` styles from the
Fomantic build.
@silverwind silverwind added topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile topic/ui Change the appearance of the Gitea UI labels Sep 6, 2021
@silverwind silverwind added this to the 1.16.0 milestone Sep 6, 2021
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Sep 6, 2021
@silverwind
Copy link
Member Author

@delvh included the rename poping to popping here in the second commit, I ran:

perl -p -i -e 's#poping#popping#g' templates/**/* web_src/js/index.js

@delvh
Copy link
Member

delvh commented Sep 7, 2021

If we are already at it: Is there a reason why these two are two separate classes?
Because as far as I can see, they could be combined into one style class pop-up.
Then everyone knows without a doubt what they are meant to do.
Doing this makes everything more readable, smaller, and less error-prone in my opinion.
Unless of course that style class is already used somewhere else…
Even though doing this might help reduce bugs in the end as those things are also already intended to popup and we then have a more unified code base…

@silverwind
Copy link
Member Author

I think that class can probably be removed entirely. I'll probably move the renaming commit to another PR as it's not directly related.

@delvh
Copy link
Member

delvh commented Sep 7, 2021

I am fine with both.
I prefer removing it entirely if possible because less code means fewer bugs…

@silverwind
Copy link
Member Author

Backed out of the renaming change here, will follow up with a separate PR later.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 7, 2021
@tomaswarynyca
Copy link
Contributor

🚀

@zeripath zeripath merged commit bc81d12 into go-gitea:main Sep 8, 2021
@silverwind silverwind deleted the rm-css-tooltip branch September 8, 2021 07:29
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants