api3dao / api3-dao-dashboard

API3 DAO dashboard
api3.eth/
14 stars 10 forks source link

DAO-188 Add Policy Select page #325

Closed mcoetzee closed 2 years ago

mcoetzee commented 2 years ago

What does this change?

How did you action this task?

Policy Select page I moved all components from src/pages/policies -> src/components/policies, to be reused by both the My Policies and Policy Select pages.

BackButton component For UX purposes, we want to prevent the BackButton from going back (via history.goBack()) if the BackButton is present on the first page the user visited (i.e. the app entry page). Otherwise if they visited a website in the same tab before opening the DAO dashboard, it would navigate back to that website. The solution was quite simple, and it was to identify the app entry page with window.history.state (the HashRouter doesn't support history/location state):

const App = () => {
  useEffect(() => {
    // When the app mounts we want to identify the current page as the one the user first visited
    // (i.e. the entry page), so that if the custom back button is present on the entry page,
    // that it knows that it can't go back.
    identifyAppEntryPage();
  }, []);
export function identifyAppEntryPage() {
  // We use session storage so that if the user refreshes the browser tab, that we don't
  // identify another entry page
  if (window.sessionStorage.getItem('hasIdentifiedAppEntryPage') !== 'true') {
    // The HashRouter (from react-router) doesn't support history/location state: https://v5.reactrouter.com/web/api/HashRouter
    window.history.replaceState({ appEntryPage: true }, '');
    window.sessionStorage.setItem('hasIdentifiedAppEntryPage', 'true');
  }
}

And then we can figure out if we can go back or not:

export default function BackButton(props: Props) {
  // We can go back if we are not on the first page the user visited
  const canGoBack = !window.history.state?.appEntryPage;

Screenshots

Screenshot 2022-09-05 at 13 24 43 Screenshot 2022-09-05 at 13 25 31
linear[bot] commented 2 years ago
DAO-188 Active Policy Select page

A new page to navigate to when the user clicks the New Claim button on the My Claims page. The page only lists the user's active policies.

Siegrift commented 2 years ago

I am 95% sure I left one more comment (but I don't see it here). I was thinking we should not have the policies components moved inside the components folder but instead have it in pages similarly to proposal-commons. My intention was that components are to be the generic building blocks for the UI and all domain specific stuff should be in pages (which is a bad name looking at it now).

mcoetzee commented 2 years ago

I was thinking we should not have the policies components moved inside the components folder but instead have it in pages similarly to proposal-commons. My intention was that components are to be the generic building blocks for the UI and all domain specific stuff should be in pages (which is a bad name looking at it now).

Yeah the lines are always a bit blurry when folder structure is the topic.

When I first started exploring the codebase, pages/proposal-commons gave the impression that it was a page itself:

pages/
  |- ...
  |- proposal-commons/
  |- proposals/

So tbh, I'm not too keen on a pages/policy-commons, but I do get what you are saying about keeping src/components more generic. Another option that I would be more keen on looks like this:

components/ (generic building blocks)
pages/
  |- ...
  |- components/ (domain specific building blocks)
    |- policies/
    |- proposal-details/
    |- proposal-list/
    |- ...
  |- my-policies/
  |- proposals/
  |- ...

wdyt?

Siegrift commented 2 years ago

When I first started exploring the codebase, pages/proposal-commons gave the impression that it was a page itself:

Yep. It initially all started with all pages bing pages (and related components were in a page subfolder), but then we started to reuse proposal specific components in multiple pages so it was awkward to have them import from another page, so I moved the common stuff (proposal-commons) alongside.

Another option that I would be more keen on looks like this:

Yeah, that feels ideal to me.

mcoetzee commented 2 years ago

Ok cool, reckon I will create a separate task for this (and keep this PR's folder structure as is so that the PR doesn't bloat)

Siegrift commented 2 years ago

Ok cool, reckon I will create a separate task for this (and keep this PR's folder structure as is so that the PR doesn't bloat)

Yeah, that seems better.