Pseudo-tabs for the currently opened page

micahdbak commented 5 months ago

You can extract the view through props passed into Navbar, ..., or create a second component.

dayvidpham commented 5 months ago

Overview:

As discussed in the W3 Committee on April 30, 2024, the currently opened page will be displayed in a "tab" in the header as shown in the image below.

Only one "tab" will exist at a time. If the user clicks on a new page, then the current tab's file name will be replaced by the new file name.
We are not implementing opening multiple tabs at once.

micahdbak commented 5 months ago

Looks good! Thanks for tackling this!

micahdbak commented 5 months ago

Oh yeah, one note, I think this component should be separate to the Navbar component, and can maybe exist in the /lib/VSCode/navigation folder. It can then be used at an app-level like so:

function App() {
  return (
    <>
      <VSCode.Navbar view={view} ... />
      <VSCode.TabView view={view} ... />
      (the actual page)
    </>
  );
}

or

function App() {
  return (
    <>
      <VSCode.Navbar view={view} ... />
      <div className='flex flex-col items-start justify-start'>
        <VSCode.Tabs view={view} ... />
        (the actual page)
      </div>
    </>
  );
}

Up to you though!

dayvidpham commented 5 months ago

Screenshots of the implementation are given in the PR!

Design issues

@harjotsk03 @micahdbak I ran into some difficulties working with the current architecture/design of the example site. Could consider opening a separate issue.

1. `VSCodeGuide` uses hardcoded margins/padding to display content around `Navbar`

Right now, the component VSCodeGuide that wraps the main content is using hardcoded CSS margin/padding values to avoid clashing with the Navbar component. For right now, I also followed suit and hardcoded some margin/padding values on the TabBar to avoid the Navbar, but I would like to use CSS Grid or Flexbox to let the browser handle this. It would be very painful to change all these values if we decide to change the dimensions of the Navbar.

// src/pages/VSCodeGuide.js
// ...
// Notice: lg:ml-80 and pt-24, lg:pt-3 in line below vvvvvvv
<div className="lg:pl-4 pt-24 lg:pt-3 lg:ml-80 lg:pr-8">
  <p className="font-300 text-white text-md ml-4 mt-3">
    Copy and paste these when you need them
  </p>
  // ...
</div>
// ...

2. Setting new search parameters causes a page refresh and re-render

Clicking on the different pages in the Navbar will cause a page refresh. This will cause all React components to not just re-render, but re-initialize from scratch. All values set through the useState, useContext, useEffect, and useMemo hooks will be lost and set back to their initial values.

About full page refreshes, I think we should use React Router mainly because all the flows with hooks are going to break on a refresh. Typically in SPAs/React, we would avoid the full page refresh with some hackery using the History API, or if we don't want to roll our own mechanism, it would be via React Router (which uses the History API).

The hackery does not feel good, but I think it is the way to go if we're using React. Below is an example of some complications that can arise.

A naive implementation is given here. Each time the user clicks a new page, there will be a full page refresh and all State values will be re-initialized. This means that view will be reset to the empty string every time a new page is loaded.

function App() {
  const [view, setView] = useState('');
  console.log("hello");
  useEffect(() => {
    const searchParams = new URLSearchParams(window.location.search);

    if (searchParams.has('view')) {
      setView(searchParams.get('view'));
    }
  }, []);

  return (
    <div className="app">
      <VSCode.Navbar />
      {/* can route using the view string with more pages */}
      <VSCode.TabBar view={view ? `${view}.py` : 'home.py'} />
      <VSCodeGuide />
    </div>
  );
}

If there is no 'view' search param, e.g. localhost:3000, then we're good and only one render occurs.

However, if there is a 'view' search param, e.g. http://localhost:3000/?view=about/whoweare, then two renders will occur.

When we click on a new page via the sidebar, a full page refresh occurs, wiping state.
Then the callback in useEffect() is run, the search param is read, and the if branch will execute and call setView(...).
setView() will cause a re-render.

This would cause our entire application to render twice just for having a search param, which would be costly. In my PR, I avoid this by simply not using the useState() hook.

Doubters can try to run the following example. Note how console.log('hello') gets called twice in the presence of a view search param (it will actually get called four times because of how React StrictMode checking works).

micahdbak commented 5 months ago

Right now, the component VSCodeGuide that wraps the main content is using hardcoded CSS margin/padding values to avoid clashing with the Navbar component. For right now, I also followed suit and hardcoded some margin/padding values on the TabBar to avoid the Navbar, but I would like to use CSS Grid or Flexbox to let the browser handle this. It would be very painful to change all these values if we decide to change the dimensions of the Navbar.

Ah, yes, we should definitely change this to work with a CSS Grid or Flexbox!

About full page refreshes, I think we should use React Router mainly because all the flows with hooks are going to break on a refresh. Typically in SPAs/React, we would avoid the full page refresh with some hackery using the History API, or if we don't want to roll our own mechanism, it would be via React Router (which uses the History API).

I'm adamant about not using the History API, as it tends to be wonky when served as compiled files (something we need to do if we want to serve static files in addition to React files). Maybe we can use hash-routing (see this React Router page); I'll look into implementing this! It shouldn't refresh the page (as anchors are intended to just jump to a part of the page, not refresh), so that would be a nice edge over search params, and it would work with static files.

Hashrouting example: sfucsss.org - Home sfucsss.org/#/about - About page sfucsss.org/#/officers - Officers page sfucsss.org/#/officers/newest - Officers (newest) sub-page ...

I'll make a task for this and implement it.

Doubters can try to run the following example. Note how console.log('hello') gets called twice in the presence of a view search param (it will actually get called four times because of how React StrictMode checking works).

Gross. OK, I'm against search params now for sure. I'll setup HashRouter as the primary routing means

micahdbak commented 5 months ago

I'm going to look over your PR now!

micahdbak commented 4 months ago

@dayvidpham , new idea for this component, over the tabs idea, which is to implement something like this:

@EarthenSky raised that having file tabs might be a little distracting (browsers already have tabs), and not very useful if we only have one tab open. A better design is a "breadcrumb" about the current page, and maybe how deep you currently are in the page (e.g., sections, etc.).

For now the breadcrumb could be as simple as displaying the file name and parent folder, such as <folder> > <filename>, and I think you can implement this statically by passing in a JSON object to the component correlating a location to a breadcrumb, something like:

import { useLocation } from 'react-router-dom';

function NavBreadcrumb(props) {
  const location = useLocation();
  const { crumbs } = props;

  const crumb = crumbs[location.pathname]; // pathname is '/', '/officers', ...
  // ...
}

<NavBreadcrumb
  crumbs={{
    "/": "README.md",
    "/officers": "about > officers.txt",
    ...
  }}
>

Thoughts?

EarthenSky commented 4 months ago

having file tabs might be a little distracting (browsers already have tabs), and not very useful if we only have one tab open

I did say this, but @micahdbak made a good point a while ago that not having file tabs makes the design look a lil wonky. I think the file tab & the breadcrumbs might be able to work together, having the breadcrumbs fill the empty area to the right of the tab.

I think having multiple open tabs would not be very helpful if the content of the page is just information about our club. If the page content is something the user is editing, or it's about multiplexing content smaller than the entire page (say a tabbed table or something embedded in the page), the affordance would be nice.

CSSS / csss-raspberry