Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
  • 04:26
    Gazook89 closed #2663
  • 04:26
    Gazook89 commented #2663
  • 03:01

    dependabot[bot] on npm_and_yarn

    (compare)

  • 03:01
    dependabot[bot] closed #2212
  • 03:01
    dependabot[bot] commented #2212
  • 03:01
    dependabot[bot] labeled #2664
  • 03:01
    dependabot[bot] opened #2664
  • 03:01

    dependabot[bot] on npm_and_yarn

    Bump nanoid from 3.3.4 to 4.0.1… (compare)

  • 02:04
    calculuschild commented #2663
  • 02:04
    calculuschild commented #2663
  • 01:54
    Gazook89 opened #2663
  • Feb 05 23:30
    ericscheid commented #2661
  • Feb 04 22:24
    5e-Cleric commented #2661
  • Feb 04 22:24
    5e-Cleric commented #2661
  • Feb 04 22:19
    5e-Cleric commented #2661
  • Feb 04 22:19
    5e-Cleric commented #2661
  • Feb 04 22:18
    5e-Cleric commented #2661
  • Feb 04 22:17
    calculuschild commented #2661
  • Feb 04 22:17
    calculuschild commented #2661
  • Feb 04 22:14
    5e-Cleric synchronize #2661
Trevor Buckner
@calculuschild
I'm tempted to say just don't show the contents in anticipation of allowing users to add more stuff onto the \page lines, possibly as folding labels (naturalcrit/homebrewery#1667) or even styling that applies to the whole page. But showing page contents might be enough for now until we make that change since I think it's still a way off.
Charlie
@jeddai
image.png
Definitely a little cleaner -- maybe some css changes we could make to the widget could help as well?
Charlie
@jeddai
But yeah we can also just leave it empty as an open/close arrow and call it a day haha
G.Ambatte
@G-Ambatte
Hacktoberfest T-shirt redeemed... We'll see how long it takes to ship.
Trevor Buckner
@calculuschild
@G-Ambatte I merged naturalcrit/homebrewery#1777 but found an error shortly after. I reverted the changes, but we can't reopen PRs so we will need to open a new one and keep tinkering on it.
G.Ambatte
@G-Ambatte
I'll open a new PR from the same branch.
Trevor Buckner
@calculuschild
perfect.
G.Ambatte
@G-Ambatte
Hmm, says there's no changes to PR.
Trevor Buckner
@calculuschild
Hm....
I reverted your PR.... there should be a difference.
G.Ambatte
@G-Ambatte
Exactly what I was thinking.
Trevor Buckner
@calculuschild
what's your branch called
G.Ambatte
@G-Ambatte
G-Ambatte:experimentalAddMetadata-#820
image.png
Trevor Buckner
@calculuschild
Mmmmmmmmm.... One moment
G.Ambatte
@G-Ambatte
Tried switching the base to PRODUCTION and back to master, no change
Trevor Buckner
@calculuschild
Oh I see what's going on. Its not comparing the file differences. It's looking at the commit ids and saying "oh i already have those commits".
My revert then applied another commit on top, but your old commits are still in the history.
G.Ambatte
@G-Ambatte
Ah ok, so we need an updated commit on the branch to create a new PR
Trevor Buckner
@calculuschild
And that commit needs to contain all of your branches changes or it will still only detect the latest commit as being different.
G.Ambatte
@G-Ambatte
Git works so well, when it's working well.
And then there's situations like this...
Trevor Buckner
@calculuschild
So we need to 1) open your branch and copy the files somewhere 2) merge master into your branch so it's up to date (or just make a a new branch) 3) copy back in your files so all your changes are there again.
Sorry this is all my bad.
G.Ambatte
@G-Ambatte
It's all good, I'll deal with it shortly... Currently plugging away at the unified EditorPage.
Trevor Buckner
@calculuschild
Sounds good.
The issue with the thumbnail was that it's pretty easy to hit the size limit for Google Drive properties (124 characters including the property name). I also foresee a similar issue when we start adding custom tags and things. We probably need to get around this by adding a new field into the document itself, similar to our css block for the style tab, but for metadata. Probably needs a whole separate PR just to set that up.
G.Ambatte
@G-Ambatte
Yeah, that sounds like a significant issue
Eric Scheid
@ericscheid_twitter
Side benefit: a downloaded brew will include all the user editable meta-data.
G.Ambatte
@G-Ambatte
Could probably slap a 100 character limit on the URL field as a temporary workaround until the metadata codefence is in place
Or just mark it to be added after the metadata codefence
The meta tags will wait - anyone who wants it desperately has already been waiting for years.
Trevor Buckner
@calculuschild
@ericscheid_twitter Yes. We would of course still have any ids (editId, shareId, probably author names, etc) hidden outside of that but most everything else we could migrate over.
Lets mark it to be added after the metadata codefence. If we implement it as a Google Drive property now, it will only make the transition to the codefence stickier because then we have to handle the case of Google brews that were saved to store the date in a property and then never updated to use the new method. We would have to handle both cases which is something we can avoid otherwise.
G.Ambatte
@G-Ambatte
Makes sense to me.
Eric Scheid
@ericscheid_twitter
Just a thought on the user page sorting - if pinched for space, dispense with the Sort Direction widget, but instead tap-again to reverse a sort. The current sort item could be indicated with a upward/downward-triangle instead of a box.
Charlie
@jeddai
Okay so I've narrowed down the thing causing the folded code blocks to unfold on any change, but I'm now getting into react lifecycle stuff and admittedly I'm very proficient with angular but react's lifecycle stuff is entirely unknown to me. Something about how the component is updating between editPage.handleTextChange setting the state (which updates the CodeEditor's props) and codeEditor.componentDidUpdateis causing the CodeMirror instance to seemingly lose all its state wrt code marks
Charlie
@jeddai
Adjusting the above, it actually looks like it's losing the state immediately after codeEditor.componentDidUpdate completes, even if none of the code in that function is run. Going to dig into the lifecycle and see what happens after that function finishes
G.Ambatte
@G-Ambatte
Odd thought about /print: what if, rather than having to load the brew and render it from Markdown, we instead passed the current output of BrewRenderer to /print?
I'm not sure how it would work, exactly, but that might eliminate the need to save /new to local storage, or to save the brew before printing to make sure that the last set of changes are still in there.
Random thought, might be worth chasing at some as-yet-undefined future point.
Trevor Buckner
@calculuschild
@G-Ambatte Posted some rambling comments on naturalcrit/homebrewery#1801 . It just addresses one issue, but I think it will clean the PR up a lot, and then I'll give it a more detailed look-over in a couple days. (Outside-the-house date night with my wife after 2 years of pandemic!)
G.Ambatte
@G-Ambatte
Enjoy it! I know we've been incredibly lucky down here; our lockdowns have been short compared to other countries, and personally, as tech support for essential services, I haven't really been locked down at all.
Trevor Buckner
@calculuschild
Our lockdown has been over for a while, but restaurants have still been closed, people aren't getting vaccinated which has made us hesitant to go out much, and getting a babysitter, etc. when everyone else has their own problems to deal with... It just has just not worked out.
Thanks though. We're excited!
G.Ambatte
@G-Ambatte
On the InitialState stuff: if we create a perfect blank/default brew in homebrew.jsx (or wherever), then _.merge that with the result of getBrewById() (probably this.props.brew in homebrew.jsx), then every Brew, passed to any Route, should be guaranteed to have all of the default Brew properties.
I think.
Probably a decent sized PR to put it into practice... I think it would modify more or less every single Page.
G.Ambatte
@G-Ambatte
Might be _.assign rather than _.merge, I forget the exact detail last time we discussed the differences.
Trevor Buckner
@calculuschild
Yeah that would probably work. And I'd you want to hold it off for a separate pr that's fine. Just make sure for the current pr we aren't going to crash if the server grabs a brew with missing data.
G.Ambatte
@G-Ambatte
Interesting update on the PDF size issue... Apparently if the image is not taller than the page, then the size does not balloon.