Skip to content

Commit

Permalink
More fixes
Browse files Browse the repository at this point in the history 
Fix focusable elements in modal
Inline string localization
Fix action bar button order and separator visibility
  • Loading branch information
@timtmok
timtmok committed Apr 2, 2024
1 parent cc70d6f commit 4216161
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 47 deletions.
1 change: 1 addition & 0 deletions src/vs/workbench/browser/positronComponents/positronModalDialog/positronModalDialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const focusableElementSelectors =
'button:not([disabled]),' +
'textarea:not([disabled]),' +
'input[type="text"]:not([disabled]),' +
'input[type="number"]:not([disabled]),' +
'input[type="radio"]:not([disabled]),' +
'input[type="checkbox"]:not([disabled]),' +
'select:not([disabled])';
Expand Down
35 changes: 15 additions & 20 deletions src/vs/workbench/contrib/positronPlots/browser/components/actionBars.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,6 @@ import { INotificationService } from 'vs/platform/notification/common/notificati
const kPaddingLeft = 14;
const kPaddingRight = 8;

/**
* Localized strings.
*/
const positronShowPreviousPlot = localize('positronShowPreviousPlot', "Show previous plot");
const positronShowNextPlot = localize('positronShowNextPlot', "Show next plot");
const positronClearAllPlots = localize('positronClearAllPlots', "Clear all plots");
const positronSavePlot = localize('positronSavePlot', "Save plot");

/**
* ActionBarsProps interface.
*/
Expand Down Expand Up @@ -72,13 +64,12 @@ export const ActionBars = (props: PropsWithChildren<ActionBarsProps>) => {

// Only show the sizing policy controls when Positron is in control of the
// sizing (i.e. don't show it on static plots)
const enableSizingPolicy = hasPlots && selectedPlot
instanceof PlotClientInstance;

const enableZoomPlot = hasPlots && selectedPlot
instanceof StaticPlotClient;
const enableSavingPlots = hasPlots &&
(selectedPlot instanceof PlotClientInstance
const enableSizingPolicy = hasPlots
&& selectedPlot instanceof PlotClientInstance;
const enableZoomPlot = hasPlots
&& selectedPlot instanceof StaticPlotClient;
const enableSavingPlots = hasPlots
&& (selectedPlot instanceof PlotClientInstance
|| selectedPlot instanceof StaticPlotClient);

useEffect(() => {
Expand Down Expand Up @@ -119,12 +110,15 @@ export const ActionBars = (props: PropsWithChildren<ActionBarsProps>) => {
<div className='action-bars'>
<PositronActionBar size='small' borderTop={true} borderBottom={true} paddingLeft={kPaddingLeft} paddingRight={kPaddingRight}>
<ActionBarRegion location='left'>
<ActionBarButton iconId='positron-left-arrow' disabled={disableLeft} tooltip={positronShowPreviousPlot} ariaLabel={positronShowPreviousPlot} onPressed={showPreviousPlotHandler} />
<ActionBarButton iconId='positron-right-arrow' disabled={disableRight} tooltip={positronShowNextPlot} ariaLabel={positronShowNextPlot} onPressed={showNextPlotHandler} />
<ActionBarButton iconId='positron-left-arrow' disabled={disableLeft} tooltip={localize('positronShowPreviousPlot', "Show previous plot")}
ariaLabel={localize('positronShowPreviousPlot', "Show previous plot")} onPressed={showPreviousPlotHandler} />
<ActionBarButton iconId='positron-right-arrow' disabled={disableRight} tooltip={localize('positronShowNextPlot', "Show next plot")}
ariaLabel={localize('positronShowNextPlot', "Show next plot")} onPressed={showNextPlotHandler} />

{(enableSizingPolicy || enableSavingPlots || enableZoomPlot) && <ActionBarSeparator />}
{enableSavingPlots && <ActionBarButton iconId='positron-save' tooltip={localize('positronSavePlot', "Save plot")}
ariaLabel={localize('positronSavePlot', "Save plot")} onPressed={savePlotHandler} />}
{enableZoomPlot && <ZoomPlotMenuButton actionHandler={zoomPlotHandler} zoomLevel={props.zoomLevel} />}
{enableSavingPlots && <ActionBarButton iconId='positron-save' tooltip={positronSavePlot} ariaLabel={positronSavePlot} onPressed={savePlotHandler} />}
{enableSizingPolicy && <ActionBarSeparator />}
{enableSizingPolicy &&
<SizingPolicyMenuButton
keybindingService={props.keybindingService}
Expand All @@ -137,7 +131,8 @@ export const ActionBars = (props: PropsWithChildren<ActionBarsProps>) => {
<ActionBarRegion location='right'>
<HistoryPolicyMenuButton plotsService={positronPlotsContext.positronPlotsService} />
<ActionBarSeparator />
<ActionBarButton iconId='clear-all' align='right' disabled={noPlots} tooltip={positronClearAllPlots} ariaLabel={positronClearAllPlots} onPressed={clearAllPlotsHandler} />
<ActionBarButton iconId='clear-all' align='right' disabled={noPlots} tooltip={localize('positronClearAllPlots', "Clear all plots")}
ariaLabel={localize('positronClearAllPlots', "Clear all plots")} onPressed={clearAllPlotsHandler} />
</ActionBarRegion>
</PositronActionBar>
</div>
Expand Down
36 changes: 11 additions & 25 deletions src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,6 @@ const SAVE_PLOT_MODAL_DIALOG_WIDTH = 500;
const SAVE_PLOT_MODAL_DIALOG_HEIGHT = 600;
const BASE_DPI = 100; // matplotlib default DPI

/**
* Localized strings.
*/
const title = localize('positronSavePlotModalDialogTitle', 'Save Plot');
const widthLabel = localize('positronSavePlotModalDialogWidth', 'Width');
const heightLabel = localize('positronSavePlotModalDialogHeight', 'Height');
const dpiLabel = localize('positronSavePlotModalDialogDPI', 'DPI');
const previewLabel = localize('positronSavePlotModalDialogUpdatePreview', 'Preview');
const noPathMessage = localize('positronSavePlotModalDialogNoPathMessage', 'Specify a path.');
const saveMessage = localize('positronSave', 'Save');
const dimensionErrorMessage = localize('positronSavePlotModalDialogDimensionError', 'Width and height must be greater than 0.');
const dpiMinMaxErrorMessage = localize('positronSavePlotModalDialogDpiMinMaxError', 'DPI must be between 1 and 300.');
const browsePlaceholderText = localize('positronSavePlotModalDialogBrowsePlaceholder', 'Save plot path');

/**
* Show the save plot modal dialog for dynamic plots.
* @param layoutService the layout service for the modal
Expand Down Expand Up @@ -143,7 +129,7 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => {

const browseHandler = async () => {
const uri = await props.fileDialogService.showSaveDialog({
title: title,
title: localize('positronSavePlotModalDialogTitle', 'Save Plot'),
filters:
[
{
Expand Down Expand Up @@ -196,7 +182,7 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => {
const previewButton = () => {
return (
<PositronButton className='button action-bar-button' onPressed={updatePreview}>
{previewLabel}
{localize('positronSavePlotModalDialogUpdatePreview', 'Preview')}
</PositronButton>
);
};
Expand All @@ -205,15 +191,15 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => {
<PositronModalDialog
width={SAVE_PLOT_MODAL_DIALOG_WIDTH}
height={SAVE_PLOT_MODAL_DIALOG_HEIGHT}
title={title}
title={localize('positronSavePlotModalDialogTitle', 'Save Plot')}
onAccept={acceptHandler}
onCancel={cancelHandler}
renderer={props.renderer}>
<ContentArea>
<div className='plot-preview-container'>
<div className='plot-preview-input'>
<div className='browse'>
<LabeledFolderInput placeholder={browsePlaceholderText} label={localize('positronSavePlotModalDialogPath', 'Path')}
<LabeledFolderInput label={localize('positronSavePlotModalDialogPath', 'Path')}
value={path.value.fsPath}
onChange={e => updatePath(e.target.value)}
onBrowse={browseHandler}
Expand All @@ -222,18 +208,18 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => {
inputRef={inputRef} />
</div>
<div className='plot-input'>
<LabeledTextInput label={widthLabel} value={width.value} type={'number'} onChange={e => updateWidth(e.target.value)} min={1} error={!width.valid} />
<LabeledTextInput label={heightLabel} value={height.value} type={'number'} onChange={e => updateHeight(e.target.value)} min={1} error={!height.valid} />
<LabeledTextInput label={dpiLabel} value={dpi.value} type={'number'} onChange={e => updateDpi(e.target.value)} min={1} max={300} error={!dpi.valid} />
<LabeledTextInput id='width' label={localize('positronSavePlotModalDialogWidth', 'Width')} value={width.value} type={'number'} onChange={e => updateWidth(e.target.value)} min={1} error={!width.valid} />
<LabeledTextInput id='height' label={localize('positronSavePlotModalDialogHeight', 'Height')} value={height.value} type={'number'} onChange={e => updateHeight(e.target.value)} min={1} error={!height.valid} />
<LabeledTextInput id='dpi' label={localize('positronSavePlotModalDialogDPI', 'DPI')} value={dpi.value} type={'number'} onChange={e => updateDpi(e.target.value)} min={1} max={300} error={!dpi.valid} />
<div className='error'>
<div>
{!path.valid && noPathMessage}
{!path.valid && localize('positronSavePlotModalDialogNoPathMessage', 'Specify a path.')}
</div>
<div>
{(!width.valid || !height.valid) && dimensionErrorMessage}
{(!width.valid || !height.valid) && localize('positronSavePlotModalDialogDimensionError', 'Width and height must be greater than 0.')}
</div>
<div>
{!dpi.valid && dpiMinMaxErrorMessage}
{!dpi.valid && localize('positronSavePlotModalDialogDpiMinMaxError', 'DPI must be between 1 and 300.')}
</div>
</div>
</div>
Expand All @@ -250,7 +236,7 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => {
</ContentArea>

<div className='plot-save-dialog-action-bar top-separator'>
<OKCancelActionBar okButtonTitle={saveMessage} onAccept={acceptHandler} onCancel={cancelHandler} preActions={previewButton} />
<OKCancelActionBar okButtonTitle={localize('positronSave', 'Save')} onAccept={acceptHandler} onCancel={cancelHandler} preActions={previewButton} />
</div>

</PositronModalDialog>
Expand Down
3 changes: 1 addition & 2 deletions src/vs/workbench/contrib/positronPlots/browser/positronPlotsService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,8 +683,7 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe

if (plot instanceof StaticPlotClient) {
// if it's a static plot, save the image to disk
const staticPlot = plot as StaticPlotClient;
uri = staticPlot.uri;
uri = plot.uri;
this.showSavePlotDialog(uri);
} else if (plot instanceof PlotClientInstance) {
// if it's a dynamic plot, present options dialog
Expand Down

0 comments on commit 4216161

Please sign in to comment.