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.

Sign up for GitHub

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

Jump to bottom

Add 'Show on a map' button to Location in profile, fix layout #26214

Merged
merged 22 commits into from
Jul 31, 2023
Merged

Add 'Show on a map' button to Location in profile, fix layout #26214

merged 22 commits into from
Jul 31, 2023

Conversation

n0toose
Copy link
Contributor

@n0toose n0toose commented Jul 29, 2023 •
edited
Loading

Not too important, but I think that it'd be a pretty neat touch.

Also fixes some layout bugs introduced by a previous PR.

@n0toose @Gusted @caesar
Add OpenStreetMap URL to Location field in profile
8fdd3ac 
Not too important, but I think that it'd be a pretty neat touch.

Also fixes a previously undiscovered visual bug when zooming in.
A separate pull request was not made because adding more elements
to the <li>'s makes the problem much more visible.

Co-authored-by: Gusted <[email protected]>
Co-authored-by: Caesar Schinas <[email protected]>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 29, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 29, 2023
@n0toose
Copy link
Contributor Author

n0toose commented Jul 29, 2023

This feature was originally developed and reviewed in the following link - this version contains some extra fixes discussed after the merge happened but were left unnoticed: https://codeberg.org/forgejo/forgejo/pulls/1076

A previous PR of mine, #23900, apparently introduced some sort of a visual bug when zooming in, and that was made slightly worse with the addition of a second icon on a profile.

P.S. Please make sure that the Co-authored-by: information will make it to the final commit.

@n0toose
Copy link
Contributor Author

n0toose commented Jul 29, 2023 •
edited
Loading

The motivation for this change is promoting other open protocols and also making these "black boxes that produce code for me" known as open-source contributors seem a bit more human by reminding people that these are people with totally different lives, a totally different background, that are leading a totally different life, yet you're still using their software and working with them: I think that a map serves the purpose of being a proper reminder for the humans that exist behind those profiles.

This change would probably benefit forges / public instances and users working in distributed groups and does not take joke entries into account.

I also intentionally avoided working with the API of OpenStreetMap as it is much less complicated for instance operators and much more privacy-respecting that way. (e.g. providing a location supplied by a user to a third party could be legally questionable in the EU)

@lunny
Copy link
Member

lunny commented Jul 29, 2023 •
edited
Loading

Since we now support README profile markdown, maybe it can be put in there?

@n0toose
Address comments by wxiaoguang
3833501 
There is still one outstanding issue.
@pull-request-size pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 29, 2023
@n0toose
Copy link
Contributor Author

n0toose commented Jul 29, 2023

Since we now support README profile markdown, maybe it can be put in there?

By the user, you mean?

@n0toose n0toose changed the title Add OpenStreetMap URL to Location field in profile Add Map URL to Location field in profile Jul 29, 2023
@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 30, 2023
@n0toose
Copy link
Contributor Author

n0toose commented Jul 30, 2023

I think I'm done here and this is ready for a review.

wxiaoguang
wxiaoguang reviewed Jul 30, 2023
View reviewed changes
custom/conf/app.example.ini Outdated Show resolved Hide resolved
options/locale/locale_en-US.ini Outdated Show resolved Hide resolved
templates/shared/user/profile_big_avatar.tmpl Outdated Show resolved Hide resolved
templates/shared/user/profile_big_avatar.tmpl Outdated Show resolved Hide resolved
web_src/css/user.css Outdated Show resolved Hide resolved
web_src/css/user.css Outdated Show resolved Hide resolved
@n0toose n0toose changed the title Add Map URL to Location field in profile Add 'Show on a map' button to Location in profile, fix layout Jul 30, 2023
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 30, 2023
@wxiaoguang wxiaoguang added this to the 1.21.0 milestone Jul 30, 2023
@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 Jul 31, 2023
@lunny lunny added type/enhancement An improvement of existing functionality reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Jul 31, 2023
@techknowlogick techknowlogick enabled auto-merge (squash) July 31, 2023 04:41
@techknowlogick techknowlogick merged commit d58c542 into go-gitea:main Jul 31, 2023
23 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 31, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 31, 2023
@zjjhot
Merge remote-tracking branch 'upstream/main'
baea445 
* upstream/main: (26 commits)
  Add 'Show on a map' button to Location in profile, fix layout (go-gitea#26214)
  Use shared template for webhook icons (go-gitea#26242)
  Reduce margins on user settings page, introduce `flex-container` (go-gitea#26046)
  Refactor and enhance issue indexer to support both searching, filtering and paging (go-gitea#26012)
  Show image size on view page (go-gitea#25884)
  Fix pull request check list is limited (go-gitea#26179)
  Fix API leaking Usermail if not logged in (go-gitea#25097)
  [skip ci] Updated licenses and gitignores
  Fix typo in metadata (go-gitea#26207)
  Update js and py dependencies (go-gitea#26243)
  De-emphasize issue sidebar buttons (go-gitea#26171)
  Don't autosize textarea in diff view (go-gitea#26233)
  Add `/public/assets` to `.ignore` (go-gitea#26232)
  Fix attachment clipboard copy on insecure origin (go-gitea#26224)
  Fix commit compare style (go-gitea#26209)
  Fix unable to display individual-level project (go-gitea#26198)
  Fix access check for org-level project (go-gitea#26182)
  Fixed incorrect locale references (go-gitea#26218)
  Use calendar icon for `Joined on...` in profiles (go-gitea#26215)
  Add changelog for 1.20.2 (go-gitea#26208)
  ...
@yp05327 yp05327 mentioned this pull request Aug 10, 2023
Merged
silverwind pushed a commit that referenced this pull request Aug 10, 2023
@yp05327
Fix the display of orgs listed in user profile (#26424)
918d0f0 
Before:

![image](https://github.com/go-gitea/gitea/assets/18380374/55759aad-cbf7-4d91-8559-b7781d184911)
After:

![image](https://github.com/go-gitea/gitea/assets/18380374/9de5136b-d0a0-4756-9426-125fd4b603d3)

In #26214, we changed each row's display into `flex` which caused this
problem.

![image](https://github.com/go-gitea/gitea/assets/18380374/66546e7e-a224-45e7-9008-224f0f2952c6)

![image](https://github.com/go-gitea/gitea/assets/18380374/e5190f88-e561-4361-894a-86c5fe88837e)
In old version:

![image](https://github.com/go-gitea/gitea/assets/18380374/36e58e98-7d75-4a63-844a-2beee460997a)

Maybe we can add paddings here?

![image](https://github.com/go-gitea/gitea/assets/18380374/dc61acd2-8995-46d4-b606-eb10c77dd877)
In old version;

![image](https://github.com/go-gitea/gitea/assets/18380374/478acba7-0bbc-4852-b208-3ae43fcea976)
@n0toose n0toose mentioned this pull request Aug 29, 2023
Merged
hazycora pushed a commit to hazycora/gitea that referenced this pull request Sep 26, 2023
@n0toose @earl-warren
[FEAT] Use OpenStreetMap in USER_LOCATION_MAP_URL by default
2a022dc 
Follow-up to d58c542 for Forgejo.

By default, Gitea does not select any map service that can be used
to introduce a 'Show this place on a map' button in the location
field of a user profile. Before I tried upstreaming this change to
Gitea, this was the case in Forgejo. This patch essentially recovers
this functionality, which is nice for public-facing instances and
communities.

Links to original PRs:
- https://codeberg.org/forgejo/forgejo/pulls/1076
- go-gitea#26214

(cherry picked from commit bb187d5f617f8efceb41810d6ff9adcaa60450bb)
(cherry picked from commit ce02ef9078a8731921caa4f7b0c1b0ac3b59a784)
(cherry picked from commit 6b75c40e2575e23810880ee8e368dc1781e2b4e4)
(cherry picked from commit 6bc8e9f5737f5721ddcd1ef5926a778a7f66a4a3)
(cherry picked from commit 063f8afdf7c2cca014c60dcc3d78d270fd236f8f)
(cherry picked from commit c5cc736b72e1ba980519e9258686f41e4eb78b42)
(cherry picked from commit 7b1bb4bedc6196fb6e8516d9a34092b16d7fab05)
wxiaoguang pushed a commit that referenced this pull request Sep 29, 2023
@n0toose
Introduce fixes and more rigorous tests for 'Show on a map' feature (#…
65e09a3 
…26803)

This change introduces a fix for the PR #26214 to set the UserLocationMapURL correctly.
@GiteaBot GiteaBot mentioned this pull request Sep 30, 2023
Merged
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Sep 30, 2023
@n0toose @GiteaBot
Introduce fixes and more rigorous tests for 'Show on a map' feature (g…
6cb29d7 
…o-gitea#26803)

This change introduces a fix for the PR go-gitea#26214 to set the UserLocationMapURL correctly.
6543 pushed a commit that referenced this pull request Sep 30, 2023
@GiteaBot @n0toose
Introduce fixes and more rigorous tests for 'Show on a map' feature (#…
de38710 
…26803) (#27365)

Backport #26803 by @n0toose

This change introduces some fixes for my original PR
(#26214) and introduces some
additional tests so that such a regression does not happen again in the
future.

Co-authored-by: Panagiotis "Ivory" Vasilopoulos <[email protected]>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Oct 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@lunny lunny lunny approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.21.0
Development

Successfully merging this pull request may close these issues.
5 participants
@n0toose @lunny @wxiaoguang @techknowlogick @GiteaBot