Progress on App.tsx - collections and publish tabs work

[andrew-polk]

---

---

This change is 

[JohnThomson]

Minor suggestions. It's encouraging that we can get this far with so little code. I wonder whether there's some way to merge this with what I did and get a working system minus at least a couple of iframes? The essence of what I did was to keep the old edit view root document and bundle as the workspace root. If we converted the pure-drawer framework to react (it's just a few divs and a lot of CSS) or could replace it with a react component), but keep the current three iframes as the content, we might be close. Not where we want to be, but it would be nice to get rid of two iframes and make app.tsx the real top of everything. And quite possibly we don't need to even render the edit iframes unless we're in that mode.

@JohnThomson reviewed 6 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on andrew-polk).

---

src/BloomBrowserUI/app/App.less line 13 at r2 (raw file):

    height: 100%;
    width: 100%;
    padding-bottom: 0 !important; // fix a dumb default coming from bloomUI

I suppose we can't just fix the dumb default because it has some purpose for the old, currently production, code?

---

src/BloomBrowserUI/app/App.tsx line 18 at r2 (raw file):

export const App: React.FunctionComponent = () => {
    const state = useWatchApiObject<{ tabStates: TabStates }>(

I'm not sure how far you're trying to go towards a desirable state. I would think if Bloom was truly a web app with only essential functionality in the backend, the 'source of truth' for which tab is active should be in react. It might need to notify C#, but it shouldn't have to ask C# or depend on notifications from C#.
This probably can't be fixed until this is in production, and maybe not for some time after that. But it might deserve a comment

---

src/BloomBrowserUI/react_components/TopBar/TopBar.tsx line 68 at r2 (raw file):

export const TopBar: React.FunctionComponent = () => {
    const state = useWatchApiObject<{ tabStates: TabStates }>(

Again, this state should be owned in typescript. We could perhaps move a step in that direction by giving this class a prop and onchange function, and letting the app own it...though that might be awkward for current production code.

[andrew-polk]

Let's talk about it!

@andrew-polk made 4 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on JohnThomson).

---

src/BloomBrowserUI/app/App.less line 13 at r2 (raw file):

Previously, JohnThomson (John Thomson) wrote…

I suppose we can't just fix the dumb default because it has some purpose for the old, currently production, code?

I inherited the rule and comment, but experimentally, I see that it is not needed. Removed.

---

src/BloomBrowserUI/app/App.tsx line 18 at r2 (raw file):

Previously, JohnThomson (John Thomson) wrote…

I'm not sure how far you're trying to go towards a desirable state. I would think if Bloom was truly a web app with only essential functionality in the backend, the 'source of truth' for which tab is active should be in react. It might need to notify C#, but it shouldn't have to ask C# or depend on notifications from C#.
This probably can't be fixed until this is in production, and maybe not for some time after that. But it might deserve a comment

Added comment. See if that addresses your question.

---

src/BloomBrowserUI/react_components/TopBar/TopBar.tsx line 68 at r2 (raw file):

Previously, JohnThomson (John Thomson) wrote…

Again, this state should be owned in typescript. We could perhaps move a step in that direction by giving this class a prop and onchange function, and letting the app own it...though that might be awkward for current production code.

I'm purposefully trying not to change anything in real Bloom code at this point. This refactor was just so App.tsx could make use of the same existing mechanism with a little shared code.

[JohnThomson]

Sure! Meanwhile, what you have seems fine.

@JohnThomson reviewed 3 files and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on andrew-polk).