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

modals should be accessible via URL #307

Closed
cvan opened this issue Apr 26, 2018 · 8 comments · Fixed by #869
Closed

modals should be accessible via URL #307

cvan opened this issue Apr 26, 2018 · 8 comments · Fixed by #869
Labels
enhancement work that enhances an existing feature good first issue

Comments

@cvan
Copy link
Contributor

cvan commented Apr 26, 2018

  1. load https://hubs.mozilla.com/
  2. scroll to footer at the bottom of the page
  3. on desktop, hover over the first three links and notice the URL tooltips are https://hubs.mozilla.com/# (from a[href="#"])

screen shot 2018-04-25 at 8 59 02 pm

ideally, at these URLs, the respective modals could be opened (or links be opened):

  • /contact - Join the Conversation
  • /subscribe - Get Updates
  • /report - Report Issues
  • /terms - Terms of Use
  • /privacy - Privacy Notice

hashes (e.g., #contact) are possible but would not be preferable, as hashes don't appear in server logs/analytics (unless manually handling them in client-side JS).

@cvan cvan added the enhancement work that enhances an existing feature label Apr 26, 2018
@gfodor
Copy link
Contributor

gfodor commented May 2, 2018

sooo is this a request to add React Router? I am not sure how to prioritize.

@cvan
Copy link
Contributor Author

cvan commented May 2, 2018

sooo is this a request to add React Router? I am not sure how to prioritize.

the History API, although confusing, can be used for this (example). React Router, under the hood, is using the History API.

what I was going to suggest is having the server return the same /index.html for each of those endpoints, and then look at window.location.pathname conditionally (in addition to the existing onClick plumbing).

here's example of using that technique. and scripts for generating the static files:

I'd be happy to help with this.

@gfodor
Copy link
Contributor

gfodor commented May 2, 2018

yep, understood -- just wondering if adding react router is the desired action item for this (and adding routes for the above dialogs) I think it makes sense but adding proper routing in general may have some incidental complexity unrelated to the case above. I think for the index page it should be fine though.

@robertlong
Copy link
Contributor

This still seems desirable. Perhaps we do need to add react router to display these modals.

@suhailsinghbains
Copy link

Hi, Can I please work on this bug ? I am new to this project.

@brianpeiris
Copy link
Contributor

Hi @suhailsinghbains. Yes, thanks, I think we do still want URLs for the dialogs listed above, unless @netpro2k or @gfodor disagree.
If you're taking this on, note that we've recently added React Router to the project, so that's what we should use for these URLs and dialogs as well: #823

@suhailsinghbains
Copy link

I was looking through the code where, I could write the Router Code, I found a file state-route.js I wasn't able to add another Route, so, I omitted the line L23, the project works just fine. Hence, I wasn't able to debug it, please help me :)

@gfodor
Copy link
Contributor

gfodor commented Feb 1, 2019

hey @suhailsinghbains thank you for offering to help with this issue! the state-router library we have in place is primarily used in the main Hub room page (hub.html) and has not been integrated into the homepage (index.html). For the homepage, it seems like we'd want to use normal URL transitions, not state oriented transitions, so a standard React Router setup in index.html would probably be just fine. If that's something you'd like to do to ensure the back button works properly on the home page we'll welcome the PR for review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement work that enhances an existing feature good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants