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

fixes: #575 links to change selected Repo #578

Merged
merged 4 commits into from
May 21, 2022

Conversation

on2onyekachi
Copy link
Contributor

@on2onyekachi on2onyekachi commented May 16, 2022

fixes: #575
Before:

Screen.Recording.2022-05-13.at.07.45.21.mov

After:

Screen.Recording.2022-05-16.at.18.40.31.mov

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • PR body includes fixes #0000-style reference to original issue #
  • ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays.

Thanks!

@TildaDares
Copy link
Member

@on2onyekachi When I tried it locally, it redirected to another URL entirely. We want only the repo to change.

Screen.Recording.2022-05-17.at.12.17.28.mov

@on2onyekachi
Copy link
Contributor Author

on2onyekachi commented May 17, 2022

I see. I also did experience this while working locally. I guess that is a result of the hard-coded URL from the config.json file. I had to change the URL pointing to my local server URL to 127.0.0.1 before actually working on it.
Screenshot 2022-05-17 at 14 58 59

Maybe we could get a solution that should allow the use of absolute URLs/URIs that work locally in a local development environment and also work on the remote server without modification.? Thanks @TildaDares

@TildaDares
Copy link
Member

@on2onyekachi Let's take the easy route and leave only the changes in index.html and configure.js. The rest can be removed. Thanks!!

@on2onyekachi
Copy link
Contributor Author

on2onyekachi commented May 17, 2022

@on2onyekachi Let's take the easy route and leave only the changes in index.html and configure.js. The rest can be removed. Thanks!!

Okay, though that's like going back to the default behavior of both links. Maybe, is there a much better approach to this?.

I also think due to the hash(#) symbol on the URL it can't possibly reload the page, hence the manual reloading of the page on the select repository dropdown buttons.
Screenshot 2022-05-17 at 21 54 40

@TildaDares, Just before I remove the other files, I will want to propose we take out that line in the configure.js file since we're only making use of it for this case only and work directly with the HTML href attribute. href="/#-all". Thereby having an absolute URL that will work on a local environment and also on a server. This should work to fix the redirecting URL and also change the selected repo to all.

@on2onyekachi
Copy link
Contributor Author

here is how it is.

ssr.mov

@TildaDares
Copy link
Member

@on2onyekachi Go for it!!

@on2onyekachi
Copy link
Contributor Author

Yeah!! Thanks, @TildaDares 🙂

@on2onyekachi
Copy link
Contributor Author

@TildaDares Done. Please review.

$('.org-wide-issues').on('click', ()=>{
location.href = $('.org-wide-issues').attr("href")
location.reload();
})
Copy link
Member

Choose a reason for hiding this comment

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

This works perfectly @on2onyekachi!! Can you just make sure that the indentation is two spaces deep? Thank you!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @TildaDares No, I followed the indentation of the file, all 4spaces. Should I fix all of it, or make it an FTO?

Copy link
Member

Choose a reason for hiding this comment

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

You know what let’s leave it that way so it doesn’t look odd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OKAYY

@on2onyekachi
Copy link
Contributor Author

Can I just fix just that? @TildaDares thanks.

@TildaDares TildaDares merged commit 148629b into publiclab:main May 21, 2022
@TildaDares
Copy link
Member

Thank you for this @on2onyekachi!!!

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.

Links doesn't change the selected repository to all.
2 participants