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.

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

3358 transcribe full screen #4151

Merged
merged 7 commits into from
Jun 25, 2024
Merged

Conversation

atbah
Copy link
Collaborator

@atbah atbah commented Jun 5, 2024

Closes #3358

@atbah atbah requested a review from benwbrum June 5, 2024 15:12
@@ -9,6 +9,8 @@
a#osd_zoom_out =svg_symbol "#icon-zoom-out", class: 'icon'
a#osd_home =svg_symbol "#icon-fit-view", class: 'icon'
a#osd_fit_width(title="#{t('.fit_width')}") =svg_symbol "#icon-fit-width", class: 'icon'
-if !@is_monitor_view
a#osd_monitor(title="#{t('.monitor')}") =svg_symbol "#icon-window", class: 'icon'
Copy link
Owner

Choose a reason for hiding this comment

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

Should we replace this with a "close window" icon when the monitor view is being displayed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, that would be great and what I had been thinking of.
I will try to change the icon using Javascript.

@benwbrum
Copy link
Owner

benwbrum commented Jun 7, 2024

It looks like something strange may be going on with sprockets. I first got an error message saying that I needed to add //= link 'transcribe.js' to app/assets/config/manifest.js.

After I did that, I see this syntax error in SCSS:

SassC::SyntaxError at /benwbrum/ai-assist-sandbox/emily-caroline-creaghe-diary-22-dec-1882-5-sept-1883-2946fa12-d861-43f8-95be-6a412b2d1286/transcribe/33267572
Error: Undefined variable: "$inputDisabledColor".
        on line 27:29 of app/assets/stylesheets/splitter.scss
>>     border-left: 2px dashed $inputDisabledColor;
   ----------------------------^
   

@benwbrum
Copy link
Owner

This seems to work great now!

Let's add the window closing icon to the monitor view (which I didn't see locally when I tested) and attempt to persist the window-drag dimensions between one page and another, the same way we do with zoom.

On the second change, the use case here is that someone generally uses their second monitor to look at the image, so they have reduced the size of the image viewer portion of the transcription screen to something minimal, so that they have more room to type. If they save a page and navigate to the next one, we don't want the to have to re-resize their transcription window again.

@atbah
Copy link
Collaborator Author

atbah commented Jun 12, 2024

okay, let me figure them out.

@atbah
Copy link
Collaborator Author

atbah commented Jun 19, 2024

@benwbrum I pushed a new commit for showing close icon. Please have a look

@benwbrum
Copy link
Owner

The close icon works really well. We still don't save the slider position, however:

  • Navigate to a transcripton page
  • Move the slider to minimize the image viewer and maximize the transcription area
  • Click on the error to the next page
  • Observe that your slider changes are lost

@atbah
Copy link
Collaborator Author

atbah commented Jun 21, 2024

Would you attach a screen record for the slider position issue?
I couldn't generate that issue on my end.

@benwbrum
Copy link
Owner

Strange, it works fine for me now. Merging.

@benwbrum benwbrum merged commit 1df3e43 into development Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Making the Transcription side of the Transcribe full screen and draggable divider
2 participants