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

MWPW-165152: Splash screen / content nav URL handling #129

Closed
wants to merge 2 commits into from
+48 −4
Closed

MWPW-165152: Splash screen / content nav URL handling #129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3aa1481
MWPW-165152: Splash screen / content nav URL handling
Axelcureno Jan 11, 2025
a7e33ee
not required check
Axelcureno Jan 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion studio/src/mas-side-nav.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { navigateToPage } from './store.js';
class MasSideNav extends LitElement {
static styles = css`
side-nav {
height: 100%;
grid-column: 1 / 2;
display: flex;
flex-direction: column;
Expand Down Expand Up @@ -68,6 +69,13 @@ class MasSideNav extends LitElement {
}
`;

navigateToSplash() {
const url = new URL(window.location.href);
Copy link
Collaborator

Choose a reason for hiding this comment

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

with the state manager, the history/url management should be consolidated into a single location as a side-effect or reaction to state update.
I think it should be in navigateToPage.

url.searchParams.set('page', 'welcome');
history.pushState({}, '', url.toString());
navigateToPage('splash')();
}

render() {
return html`<side-nav>
<div class="dropdown-container">
Expand All @@ -77,7 +85,7 @@ class MasSideNav extends LitElement {
<sp-sidenav-item
label="Home"
value="home"
@click="${navigateToPage('splash')}"
@click="${this.navigateToSplash}"
selected
>
<sp-icon-home slot="icon"></sp-icon-home>
Expand Down
9 changes: 8 additions & 1 deletion studio/src/mas-splash-screen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ class MasSplashScreen extends LitElement {
openOfferSelectorTool();
}

navigateToContent() {
const url = new URL(window.location.href);
url.searchParams.delete('page');
history.replaceState({}, '', url.toString());
navigateToPage('content')();
}

render() {
return html`<div id="splash-container">
<h1>Welcome, ${this.userName}</h1>
Expand All @@ -39,7 +46,7 @@ class MasSplashScreen extends LitElement {
<div class="actions-grid">
<div
class="quick-action-card"
@click=${navigateToPage('content')}
@click=${this.navigateToContent}
heading="Go to Content"
>
<div slot="cover-photo">${contentIcon}</div>
Expand Down
12 changes: 11 additions & 1 deletion studio/src/store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ import { reactiveStore } from './reactivity/reactive-store.js';
const initialSearch = MasSearch.fromHash();
const initialFilters = MasFilters.fromHash();

const url = new URL(window.location.href);
const pageParam = url.searchParams.get('page');

const initialPage =
pageParam === 'welcome'
Copy link
Collaborator

Choose a reason for hiding this comment

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

in the absence of search query in the URL, the pageParam should be set to welcome so that the welcome page has always the same url.

? 'splash'
: initialSearch.path
? 'content'
: 'splash';

const Store = {
fragments: {
list: {
Expand All @@ -31,7 +41,7 @@ const Store = {
), // 'render' | 'table'
selecting: reactiveStore(false),
selection: reactiveStore([]),
currentPage: reactiveStore(initialSearch.query ? 'content' : 'splash'), // 'splash' | 'content'
currentPage: reactiveStore(initialPage), // 'splash' | 'content'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think since we already have a store for the current page, we can just link it to the hash, just as search and filters are, and rename splash to welcome. The only downside will be that we always have the page hash param in the url, but on the other hand it will be more consistent and easier to handle.

Right now we have the currentPage store that handles page navigation, but it's not directly linked to the query params, we have an intermediary who checks for welcome value and sets splash in the store. I say we just "cut the middle man".

This will also remove the need to handle history manually. @yesil mentioned history can be managed in navigateToPage, and I agree, but I think if we link the hash to the store this is no longer needed, the browser will do it for you.

};

export default Store;
Expand Down
21 changes: 20 additions & 1 deletion studio/src/studio.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import './mas-toast.js';
import './mas-hash-manager.js';
import './mas-splash-screen.js';
import StoreController from './reactivity/store-controller.js';
import Store from './store.js';
import Store, { navigateToPage } from './store.js';

const BUCKET_TO_ENV = {
e155390: 'qa',
Expand Down Expand Up @@ -56,6 +56,25 @@ class MasStudio extends LitElement {
></mas-splash-screen>`;
}

connectedCallback() {
super.connectedCallback();
}

handleInitialPageLoad() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

handleInitialPageLoad doesn't seem to be used.

const url = new URL(window.location.href);
const pageParam = url.searchParams.get('page');

if (pageParam === 'welcome') {
navigateToPage('splash')();
history.pushState({}, '', url.toString());
return;
} else {
url.searchParams.delete('page');
history.replaceState({}, '', url.toString());
navigateToPage('content')();
}
}

render() {
return html`
<mas-top-nav env="${this.env}"></mas-top-nav>
Expand Down
Loading